feat: Path-based component resolution for all commands#1774
Conversation
…cker, describe, and validate Enables developers to use filesystem paths (e.g., `.`, `./vpc`, `components/terraform/vpc`) instead of component names with all component-related commands. Paths are automatically resolved to component names based on stack configuration, eliminating the need to memorize component names and improving developer experience. Supports all terraform, helmfile, packer, describe, and validate operations. Includes: - Core path extraction and normalization utilities with symlink resolution - Stack validation to ensure component exists in specified stack - Enhanced tab completion with path support and graceful degradation - Comprehensive integration tests (17 scenarios) and unit tests (6 test cases) - Full documentation with examples for all command types - Blog post for release notes Changes are backward compatible - existing component name syntax continues to work. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds path-based component resolution across CLI commands: detects path-like component args, resolves filesystem paths to component names via a new Resolver and stack loader, surfaces richer path-related errors, converts several exit-on-error flows to error returns, and populates absolute base/stack paths in config; includes tests, fixtures, docs, and completion updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant CmdUtils
participant Config
participant Resolver
participant Stacks
User->>CLI: run command with component arg (name or path)
CLI->>CmdUtils: getConfigAndStacksInfo(cmd, args)
CmdUtils->>Config: Load AtmosConfig (populate BasePathAbsolute)
Config-->>CmdUtils: AtmosConfiguration
alt component arg is path-like (NeedsPathResolution)
CmdUtils->>Resolver: ResolveComponentFromPathWithoutTypeCheck(path, stack?)
Resolver->>Config: normalize path, resolve symlinks, map to component dirs
Resolver->>Stacks: if stack provided, validate component exists in stack
Stacks-->>Resolver: match / not found / ambiguous
Resolver-->>CmdUtils: resolved component name or detailed error
else component arg is name
CmdUtils->>Stacks: find component by name across stacks
Stacks-->>CmdUtils: match / ambiguous / not found
end
CmdUtils-->>CLI: ConfigAndStacksInfo (resolved component or error)
CLI->>CLI: proceed with execution or surface error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned Files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/describe_component.go (1)
76-107: Handle path detection cross-platform.Loving the new resolver, but this guard never fires on Windows.
filepath.Separatorthere is\, so a very common input like./vpcorcomponents/terraform/vpc(both using/) leavesneedsPathResolutionfalse, and we end up searching for a component literally named./vpc. That breaks every path-based workflow on Windows. Please broaden the check—e.g., look for both'/'and'\\', or lean onstrings.ContainsAny(component, "/\\")in addition tofilepath.IsAbs(component)—so the detection works everywhere.
🧹 Nitpick comments (2)
cmd/cmd_utils.go (1)
871-888: Consider the default component type assumption.Defaulting to "terraform" when the component type can't be determined may cause confusion if users work with helmfile or packer more frequently. However, since terraform is the most common component type in Atmos, this is a reasonable default.
Consider logging a trace message when falling back to the default to aid debugging.
func determineComponentTypeFromCommand(cmd *cobra.Command) string { // Walk up to find the root component command (terraform, helmfile, packer). current := cmd for current != nil { switch current.Name() { case "terraform": return "terraform" case "helmfile": return "helmfile" case "packer": return "packer" } current = current.Parent() } // Default to terraform if we can't determine + log.Trace("Could not determine component type from command chain, defaulting to terraform") return "terraform" }internal/exec/component_resolver_test.go (1)
15-82: Consider expanding test coverage forResolveComponentFromPath.The current test only covers the error case (path not in component directory). While the skip logic for minimal test environments is appropriate, consider adding table-driven test cases for successful resolution scenarios when the environment supports it. This would improve confidence in the happy path behavior.
📜 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 (16)
cmd/cmd_utils.go(4 hunks)cmd/describe_component.go(3 hunks)cmd/validate_component.go(2 hunks)docs/prd/component-path-resolution.md(1 hunks)errors/errors.go(1 hunks)internal/exec/cli_utils.go(2 hunks)internal/exec/component_resolver.go(1 hunks)internal/exec/component_resolver_test.go(1 hunks)pkg/schema/schema.go(2 hunks)pkg/utils/component_reverse_path_utils.go(1 hunks)pkg/utils/component_reverse_path_utils_test.go(1 hunks)tests/test-cases/component-path-resolution.yaml(1 hunks)tests/test-cases/tab-completions.yaml(1 hunks)website/blog/2025-11-08-path-based-component-resolution.mdx(1 hunks)website/docs/cli/commands/describe/describe-component.mdx(3 hunks)website/docs/cli/commands/validate/validate-component.mdx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/schema/schema.gopkg/utils/component_reverse_path_utils.gopkg/utils/component_reverse_path_utils_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
Files:
pkg/schema/schema.gointernal/exec/component_resolver_test.gocmd/describe_component.gocmd/validate_component.gopkg/utils/component_reverse_path_utils.gointernal/exec/component_resolver.gopkg/utils/component_reverse_path_utils_test.goerrors/errors.gointernal/exec/cli_utils.gocmd/cmd_utils.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/schema/schema.gocmd/describe_component.gocmd/validate_component.gopkg/utils/component_reverse_path_utils.gointernal/exec/component_resolver.goerrors/errors.gointernal/exec/cli_utils.gocmd/cmd_utils.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
Files:
internal/exec/component_resolver_test.gopkg/utils/component_reverse_path_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
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
Files:
cmd/describe_component.gocmd/validate_component.gocmd/cmd_utils.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/docs/cli/commands/describe/describe-component.mdxwebsite/docs/cli/commands/validate/validate-component.mdxwebsite/blog/2025-11-08-path-based-component-resolution.mdx
🧠 Learnings (69)
📓 Common learnings
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: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
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: 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: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/path_utils.go:145-146
Timestamp: 2024-10-23T22:11:41.077Z
Learning: In the `atmos` project, the preference is to print relative paths in log messages instead of full paths.
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-02-04T22:45:15.845Z
Learning: URL validation in Atmos should support query parameters (like `depth=1` and `ref=`) which are commonly used in Git URLs for shallow clones and version references.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos-cli-imports/atmos.yaml:8-8
Timestamp: 2025-01-25T15:21:40.413Z
Learning: In Atmos, when a directory is specified for configuration loading (e.g., in the `import` section of atmos.yaml), all files within that directory should be treated as Atmos configurations. Do not suggest restricting file extensions in directory-based glob patterns.
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-19T14:50:16.194Z
Learning: In the Atmos project, path traversal is acceptable due to its role in automation. Do not flag path traversal as an issue in code reviews.
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.
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.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
📚 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:
pkg/schema/schema.gointernal/exec/component_resolver_test.gocmd/describe_component.gocmd/validate_component.gopkg/utils/component_reverse_path_utils.gointernal/exec/component_resolver.goerrors/errors.gowebsite/docs/cli/commands/validate/validate-component.mdxcmd/cmd_utils.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
tests/test-cases/tab-completions.yamlinternal/exec/component_resolver_test.gocmd/validate_component.gopkg/utils/component_reverse_path_utils_test.godocs/prd/component-path-resolution.mdtests/test-cases/component-path-resolution.yaml
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
tests/test-cases/tab-completions.yamlinternal/exec/component_resolver_test.gopkg/utils/component_reverse_path_utils_test.godocs/prd/component-path-resolution.mdtests/test-cases/component-path-resolution.yaml
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
tests/test-cases/tab-completions.yamlinternal/exec/component_resolver_test.gopkg/utils/component_reverse_path_utils_test.godocs/prd/component-path-resolution.mdtests/test-cases/component-path-resolution.yaml
📚 Learning: 2025-01-25T15:21:40.413Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos-cli-imports/atmos.yaml:8-8
Timestamp: 2025-01-25T15:21:40.413Z
Learning: In Atmos, when a directory is specified for configuration loading (e.g., in the `import` section of atmos.yaml), all files within that directory should be treated as Atmos configurations. Do not suggest restricting file extensions in directory-based glob patterns.
Applied to files:
tests/test-cases/tab-completions.yaml
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/test-cases/tab-completions.yamltests/test-cases/component-path-resolution.yaml
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Include integration tests for command flows and end-to-end CLI where possible
Applied to files:
tests/test-cases/tab-completions.yaml
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
internal/exec/component_resolver_test.gopkg/utils/component_reverse_path_utils_test.gotests/test-cases/component-path-resolution.yaml
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
internal/exec/component_resolver_test.gopkg/utils/component_reverse_path_utils_test.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/component_resolver_test.gocmd/describe_component.gocmd/validate_component.gopkg/utils/component_reverse_path_utils.gointernal/exec/component_resolver.gopkg/utils/component_reverse_path_utils_test.goerrors/errors.godocs/prd/component-path-resolution.mdtests/test-cases/component-path-resolution.yamlinternal/exec/cli_utils.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests. This is implemented through custom testing framework extensions and is used consistently throughout the test suite.
Applied to files:
internal/exec/component_resolver_test.gopkg/utils/component_reverse_path_utils_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 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/component_resolver_test.gopkg/utils/component_reverse_path_utils_test.gointernal/exec/cli_utils.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests.
Applied to files:
internal/exec/component_resolver_test.gopkg/utils/component_reverse_path_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:
internal/exec/component_resolver_test.gocmd/describe_component.gowebsite/docs/cli/commands/describe/describe-component.mdxcmd/validate_component.gointernal/exec/component_resolver.gowebsite/docs/cli/commands/validate/validate-component.mdxtests/test-cases/component-path-resolution.yamlcmd/cmd_utils.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method that can be called on *testing.T objects. This functionality is implemented through custom testing framework extensions and is used consistently throughout the test suite for changing working directories during tests.
Applied to files:
internal/exec/component_resolver_test.gopkg/utils/component_reverse_path_utils_test.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/component_resolver_test.gointernal/exec/component_resolver.gopkg/utils/component_reverse_path_utils_test.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
internal/exec/component_resolver_test.gocmd/validate_component.gointernal/exec/component_resolver.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
internal/exec/component_resolver_test.gopkg/utils/component_reverse_path_utils_test.go
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
internal/exec/component_resolver_test.gointernal/exec/component_resolver.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands
Applied to files:
cmd/describe_component.gocmd/validate_component.gointernal/exec/cli_utils.gocmd/cmd_utils.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Provide clear error messages to users and troubleshooting hints where appropriate
Applied to files:
cmd/describe_component.gointernal/exec/cli_utils.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Include examples in Cobra command help
Applied to files:
cmd/describe_component.gointernal/exec/cli_utils.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/describe_component.gointernal/exec/cli_utils.gocmd/cmd_utils.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Viper for managing configuration, environment variables, and flags in the CLI
Applied to files:
cmd/describe_component.gocmd/validate_component.gointernal/exec/cli_utils.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*.go : Follow Go error handling idioms and use meaningful error messages
Applied to files:
cmd/describe_component.gointernal/exec/cli_utils.go
📚 Learning: 2025-02-03T06:00:11.419Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.
Applied to files:
cmd/describe_component.gointernal/exec/cli_utils.go
📚 Learning: 2025-01-18T15:15:41.645Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.
Applied to files:
cmd/describe_component.goerrors/errors.gointernal/exec/cli_utils.go
📚 Learning: 2025-11-08T19:56:18.647Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.647Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
cmd/describe_component.gocmd/validate_component.gopkg/utils/component_reverse_path_utils_test.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
cmd/describe_component.gocmd/validate_component.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:
cmd/describe_component.gocmd/validate_component.gocmd/cmd_utils.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
cmd/describe_component.gocmd/validate_component.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:
cmd/describe_component.gocmd/validate_component.gointernal/exec/component_resolver.goerrors/errors.go
📚 Learning: 2025-09-07T18:07:00.549Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-01-19T15:49:15.593Z
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.
Applied to files:
website/docs/cli/commands/describe/describe-component.mdxwebsite/docs/cli/commands/validate/validate-component.mdxwebsite/blog/2025-11-08-path-based-component-resolution.mdx
📚 Learning: 2025-09-24T20:45:40.401Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: tests/fixtures/scenarios/atmos-auth/stacks/deploy/nonprod.yaml:3-4
Timestamp: 2025-09-24T20:45:40.401Z
Learning: In Atmos stack files, the correct syntax for importing other stack files is `import:` (singular), not `imports:` (plural). All stack files in the Atmos codebase consistently use `import:` followed by a list of paths to import.
Applied to files:
website/docs/cli/commands/describe/describe-component.mdx
📚 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:
website/docs/cli/commands/describe/describe-component.mdxinternal/exec/cli_utils.go
📚 Learning: 2024-10-21T17:51:53.976Z
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.
Applied to files:
website/docs/cli/commands/describe/describe-component.mdxwebsite/docs/cli/commands/validate/validate-component.mdx
📚 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/validate_component.gointernal/exec/component_resolver.gointernal/exec/cli_utils.gocmd/cmd_utils.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:
cmd/validate_component.gointernal/exec/component_resolver.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/validate_component.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
cmd/validate_component.gointernal/exec/cli_utils.go
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
cmd/validate_component.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/component_resolver.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
internal/exec/component_resolver.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/component_resolver.gointernal/exec/cli_utils.go
📚 Learning: 2025-10-08T06:48:07.499Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1602
File: internal/exec/stack_processor_utils.go:968-1003
Timestamp: 2025-10-08T06:48:07.499Z
Learning: The `FindComponentDependenciesLegacy` function in `internal/exec/stack_processor_utils.go` is legacy code that is not actively used and is kept only for backward compatibility purposes.
Applied to files:
internal/exec/component_resolver.gocmd/cmd_utils.go
📚 Learning: 2025-04-26T15:54:10.506Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 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/component_resolver.goerrors/errors.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
errors/errors.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
errors/errors.go
📚 Learning: 2025-10-02T19:17:51.630Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1504
File: pkg/profiler/profiler.go:20-31
Timestamp: 2025-10-02T19:17:51.630Z
Learning: In pkg/profiler/profiler.go, profiler-specific errors (ErrUnsupportedProfileType, ErrStartCPUProfile, ErrStartTraceProfile, ErrCreateProfileFile) must remain local and cannot be moved to errors/errors.go due to an import cycle: pkg/profiler → errors → pkg/schema → pkg/profiler. This is a valid exception to the centralized errors policy.
Applied to files:
errors/errors.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
errors/errors.go
📚 Learning: 2024-12-05T22:33:54.807Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_stacks.go:55-56
Timestamp: 2024-12-05T22:33:54.807Z
Learning: In the atmos project, the `u.LogErrorAndExit` function logs the error and exits the command execution appropriately within flag completion functions.
Applied to files:
errors/errors.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
docs/prd/component-path-resolution.md
📚 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:
docs/prd/component-path-resolution.mdinternal/exec/cli_utils.gocmd/cmd_utils.go
📚 Learning: 2024-11-22T12:38:33.132Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Applied to files:
docs/prd/component-path-resolution.md
📚 Learning: 2024-11-25T17:17:15.703Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Applied to files:
tests/test-cases/component-path-resolution.yaml
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
tests/test-cases/component-path-resolution.yaml
📚 Learning: 2024-10-23T22:11:41.077Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/path_utils.go:145-146
Timestamp: 2024-10-23T22:11:41.077Z
Learning: In the `atmos` project, the preference is to print relative paths in log messages instead of full paths.
Applied to files:
website/blog/2025-11-08-path-based-component-resolution.mdx
📚 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/cli_utils.gocmd/cmd_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:
internal/exec/cli_utils.gocmd/cmd_utils.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use kebab-case for command-line flags
Applied to files:
internal/exec/cli_utils.gocmd/cmd_utils.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*.go : Wrap errors with context using fmt.Errorf("context: %w", err)
Applied to files:
internal/exec/cli_utils.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Provide comprehensive help text for all commands and flags
Applied to files:
cmd/cmd_utils.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/cmd_utils.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/cmd_utils.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/cmd_utils.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 never returns an error as it simply stores the completion function in an internal map. Error handling for this method call is unnecessary.
Applied to files:
cmd/cmd_utils.go
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1261
File: internal/exec/utils.go:639-640
Timestamp: 2025-05-22T15:42:10.906Z
Learning: In the Atmos codebase, when appending slices with `args := append(configAndStacksInfo.CliArgs, configAndStacksInfo.AdditionalArgsAndFlags...)`, it's intentional that the result is not stored back in the original slice. This pattern is used when the merged result serves a different purpose than the original slices, such as when creating a filtered version for component section assignments.
Applied to files:
cmd/cmd_utils.go
🧬 Code graph analysis (7)
internal/exec/component_resolver_test.go (3)
pkg/schema/schema.go (5)
AtmosConfiguration(27-65)Components(375-385)Terraform(335-347)Helmfile(361-368)Packer(370-373)internal/exec/component_resolver.go (2)
ResolveComponentFromPath(34-79)ResolveComponentFromPathWithoutTypeCheck(103-136)errors/errors.go (2)
ErrComponentTypeMismatch(462-462)ErrPathResolutionFailed(464-464)
cmd/describe_component.go (1)
internal/exec/component_resolver.go (1)
ResolveComponentFromPathWithoutTypeCheck(103-136)
cmd/validate_component.go (3)
pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (1)
ConfigAndStacksInfo(546-630)internal/exec/component_resolver.go (1)
ResolveComponentFromPathWithoutTypeCheck(103-136)
pkg/utils/component_reverse_path_utils.go (4)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (3)
ErrPathResolutionFailed(464-464)ErrPathNotInComponentDir(461-461)ErrPathIsComponentBase(465-465)pkg/logger/log.go (2)
Debug(24-26)Trace(14-16)
internal/exec/component_resolver.go (4)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/utils/component_reverse_path_utils.go (1)
ExtractComponentInfoFromPath(30-62)errors/errors.go (3)
ErrPathResolutionFailed(464-464)ErrComponentTypeMismatch(462-462)ErrComponentNotInStack(463-463)internal/exec/utils.go (1)
FindStacksMap(245-295)
pkg/utils/component_reverse_path_utils_test.go (2)
pkg/schema/schema.go (4)
AtmosConfiguration(27-65)Components(375-385)Terraform(335-347)Helmfile(361-368)pkg/utils/component_reverse_path_utils.go (2)
ComponentInfo(16-21)ExtractComponentInfoFromPath(30-62)
cmd/cmd_utils.go (6)
pkg/config/config.go (1)
InitCliConfig(25-62)errors/error_funcs.go (1)
CheckErrorPrintAndExit(75-98)errors/errors.go (1)
ErrPathResolutionFailed(464-464)internal/exec/component_resolver.go (1)
ResolveComponentFromPath(34-79)pkg/logger/log.go (2)
Debug(24-26)Trace(14-16)pkg/schema/schema.go (1)
ConfigAndStacksInfo(546-630)
🪛 LanguageTool
docs/prd/component-path-resolution.md
[grammar] ~29-~29: Please add a punctuation mark at the end of paragraph.
Context: ...he command using the resolved component name ### User Experience ```bash # New cap...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~159-~159: Consider using a typographic opening quote here.
Context: ... not within any component base → Error: "Path is not within Atmos component direc...
(EN_QUOTES)
[typographical] ~159-~159: Consider using a typographic close quote here.
Context: ...s not within Atmos component directories" - Path equals component base → Error: "...
(EN_QUOTES)
[typographical] ~160-~160: Consider using a typographic opening quote here.
Context: ..." - Path equals component base → Error: "Must specify a component directory, not ...
(EN_QUOTES)
[typographical] ~160-~160: Consider using a typographic close quote here.
Context: ...ponent directory, not the base directory" - Cannot determine component type → Err...
(EN_QUOTES)
[typographical] ~161-~161: Consider using a typographic opening quote here.
Context: ...annot determine component type → Error: "Could not determine component type from ...
(EN_QUOTES)
[typographical] ~161-~161: Consider using a typographic close quote here.
Context: ...d not determine component type from path" Estimated Effort: 50-100 lines, 2-...
(EN_QUOTES)
[typographical] ~702-~702: In American English, use a period after an abbreviation.
Context: .... Cross-platform path handling (Windows vs Unix) 6. Tab completion with path argum...
(MISSING_PERIOD_AFTER_ABBREVIATION)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/component-path-resolution.md
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
123-123: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
294-294: Hard tabs
Column: 1
(MD010, no-hard-tabs)
295-295: Hard tabs
Column: 1
(MD010, no-hard-tabs)
296-296: Hard tabs
Column: 1
(MD010, no-hard-tabs)
298-298: Hard tabs
Column: 1
(MD010, no-hard-tabs)
299-299: Hard tabs
Column: 1
(MD010, no-hard-tabs)
300-300: Hard tabs
Column: 1
(MD010, no-hard-tabs)
301-301: Hard tabs
Column: 1
(MD010, no-hard-tabs)
302-302: Hard tabs
Column: 1
(MD010, no-hard-tabs)
303-303: Hard tabs
Column: 1
(MD010, no-hard-tabs)
304-304: Hard tabs
Column: 1
(MD010, no-hard-tabs)
305-305: Hard tabs
Column: 1
(MD010, no-hard-tabs)
306-306: Hard tabs
Column: 1
(MD010, no-hard-tabs)
307-307: Hard tabs
Column: 1
(MD010, no-hard-tabs)
308-308: Hard tabs
Column: 1
(MD010, no-hard-tabs)
309-309: Hard tabs
Column: 1
(MD010, no-hard-tabs)
310-310: Hard tabs
Column: 1
(MD010, no-hard-tabs)
311-311: Hard tabs
Column: 1
(MD010, no-hard-tabs)
312-312: Hard tabs
Column: 1
(MD010, no-hard-tabs)
313-313: Hard tabs
Column: 1
(MD010, no-hard-tabs)
314-314: Hard tabs
Column: 1
(MD010, no-hard-tabs)
315-315: Hard tabs
Column: 1
(MD010, no-hard-tabs)
316-316: Hard tabs
Column: 1
(MD010, no-hard-tabs)
317-317: Hard tabs
Column: 1
(MD010, no-hard-tabs)
319-319: Hard tabs
Column: 1
(MD010, no-hard-tabs)
320-320: Hard tabs
Column: 1
(MD010, no-hard-tabs)
321-321: Hard tabs
Column: 1
(MD010, no-hard-tabs)
322-322: Hard tabs
Column: 1
(MD010, no-hard-tabs)
323-323: Hard tabs
Column: 1
(MD010, no-hard-tabs)
324-324: Hard tabs
Column: 1
(MD010, no-hard-tabs)
326-326: Hard tabs
Column: 1
(MD010, no-hard-tabs)
327-327: Hard tabs
Column: 1
(MD010, no-hard-tabs)
328-328: Hard tabs
Column: 1
(MD010, no-hard-tabs)
329-329: Hard tabs
Column: 1
(MD010, no-hard-tabs)
330-330: Hard tabs
Column: 1
(MD010, no-hard-tabs)
331-331: Hard tabs
Column: 1
(MD010, no-hard-tabs)
354-354: Hard tabs
Column: 1
(MD010, no-hard-tabs)
355-355: Hard tabs
Column: 1
(MD010, no-hard-tabs)
356-356: Hard tabs
Column: 1
(MD010, no-hard-tabs)
357-357: Hard tabs
Column: 1
(MD010, no-hard-tabs)
358-358: Hard tabs
Column: 1
(MD010, no-hard-tabs)
359-359: Hard tabs
Column: 1
(MD010, no-hard-tabs)
361-361: Hard tabs
Column: 1
(MD010, no-hard-tabs)
362-362: Hard tabs
Column: 1
(MD010, no-hard-tabs)
363-363: Hard tabs
Column: 1
(MD010, no-hard-tabs)
364-364: Hard tabs
Column: 1
(MD010, no-hard-tabs)
365-365: Hard tabs
Column: 1
(MD010, no-hard-tabs)
366-366: Hard tabs
Column: 1
(MD010, no-hard-tabs)
367-367: Hard tabs
Column: 1
(MD010, no-hard-tabs)
369-369: Hard tabs
Column: 1
(MD010, no-hard-tabs)
786-786: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (9)
- GitHub Check: Build (windows)
- GitHub Check: Build (macos)
- GitHub Check: Analyze (go)
- GitHub Check: Review Dependency Licenses
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Run pre-commit hooks
- GitHub Check: autofix
- GitHub Check: Summary
Fixed MDX syntax errors in describe and validate component documentation by converting markdown lists to proper HTML structure inside <dd> tags. - Replaced markdown list syntax (- item) with <ul>/<li> HTML tags - Converted inline code backticks to <code> tags inside HTML - Ensured proper HTML structure for multi-line content in <dd> elements Fixes CI build errors: - Expected a closing tag for `<dd>` (138:3-138:7) in describe-component.mdx - Expected a closing tag for `<dd>` (93:3-93:7) in validate-component.mdx 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Previously, ResolveComponentFromPath always returned the filesystem-based component name (componentInfo.FullComponent) even after validateComponentInStack confirmed the component existed in the stack. This caused issues when the stack used a different key (alias) via the 'component' or 'metadata.component' fields. Changes: - Updated validateComponentInStack to return (string, error) instead of just error - Implemented alias lookup logic that checks: 1. Direct key match in stack configuration 2. Alias via 'component' field 3. Alias via 'metadata.component' field - Updated ResolveComponentFromPath to capture and return the actual stack key - Updated ResolveComponentFromPathWithoutTypeCheck similarly This ensures that subsequent operations use the correct stack key to access component configuration, fixing cases where stack key != filesystem path. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Previously, any component argument containing a path separator (like "components/vpc" or "vpc/security-group") was treated as a filesystem path, causing path resolution to be attempted. This broke for component names that legitimately contain slashes. Changes: - Added IsExplicitComponentPath() helper function in internal/exec/cli_utils.go - Only treat arguments as paths if they: - Are "." (current directory), OR - Start with "./" or "../" (relative path), OR - Start with "/" (absolute path) - Otherwise, treat as component name (even if contains slashes) - Updated describe component, validate component, and terraform/helmfile/packer commands to use the shared helper This fixes the issue where users couldn't use component names like "vpc" when a directory like "components/terraform/vpc" existed, and ensures consistent behavior across all commands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…elper Moved the IsExplicitComponentPath helper function to a new pkg/component package to better organize component-related utilities and reduce bloat in internal/exec. This is a first step toward creating a focused component package that will eventually house component registry, resolver, and other component-related functionality. Changes: - Created pkg/component/path.go with IsExplicitComponentPath helper - Updated all imports to use comp alias to avoid conflict with "component" variables - Component resolver and validator remain in internal/exec to avoid circular dependencies This follows the architectural pattern of creating purpose-built packages instead of adding everything to pkg/utils. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
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.
📒 Files selected for processing (15)
cmd/describe_component.go(2 hunks)cmd/validate_component.go(2 hunks)go.mod(1 hunks)internal/exec/cli_utils.go(2 hunks)internal/exec/component_resolver.go(1 hunks)pkg/component/atmos.yaml(0 hunks)pkg/component/component_processor.go(0 hunks)pkg/component/component_processor_test.go(0 hunks)pkg/component/mock/config.go(0 hunks)pkg/component/mock/mock.go(0 hunks)pkg/component/mock/mock_test.go(0 hunks)pkg/component/path.go(1 hunks)pkg/component/provider.go(0 hunks)pkg/component/registry.go(0 hunks)pkg/component/registry_test.go(0 hunks)
💤 Files with no reviewable changes (9)
- pkg/component/mock/config.go
- pkg/component/provider.go
- pkg/component/component_processor.go
- pkg/component/component_processor_test.go
- pkg/component/mock/mock.go
- pkg/component/atmos.yaml
- pkg/component/registry_test.go
- pkg/component/registry.go
- pkg/component/mock/mock_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/validate_component.go
- internal/exec/cli_utils.go
🧰 Additional context used
📓 Path-based instructions (5)
go.{mod,sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
go.{mod,sum}: Manage dependencies with Go modules
Keep dependencies up to date
Files:
go.mod
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/component/path.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
Files:
pkg/component/path.gointernal/exec/component_resolver.gocmd/describe_component.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/component/path.gointernal/exec/component_resolver.gocmd/describe_component.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
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
Files:
cmd/describe_component.go
🧠 Learnings (31)
📓 Common learnings
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: 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: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to go.{mod,sum} : Manage dependencies with Go modules
Applied to files:
go.mod
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to go.{mod,sum} : Keep dependencies up to date
Applied to files:
go.mod
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
go.mod
📚 Learning: 2025-03-12T21:38:42.699Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1139
File: pkg/config/go-homedir/homedir.go:183-196
Timestamp: 2025-03-12T21:38:42.699Z
Learning: The code in pkg/config/go-homedir is a direct fork of the mitchellh/go-homedir package and was intentionally imported as-is without modifications to maintain consistency with the original source. Security concerns or other improvements may be addressed in future PRs.
Applied to files:
go.mod
📚 Learning: 2024-10-23T20:13:23.054Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 731
File: pkg/utils/file_utils.go:198-202
Timestamp: 2024-10-23T20:13:23.054Z
Learning: In `pkg/utils/file_utils.go`, the current implementation of the `IsURL` function is considered sufficient; avoid suggesting more complex URL validation in future reviews.
Applied to files:
pkg/component/path.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:
pkg/component/path.gointernal/exec/component_resolver.gocmd/describe_component.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:
internal/exec/component_resolver.gocmd/describe_component.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:
internal/exec/component_resolver.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/component_resolver.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/component_resolver.go
📚 Learning: 2025-10-08T06:48:07.499Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1602
File: internal/exec/stack_processor_utils.go:968-1003
Timestamp: 2025-10-08T06:48:07.499Z
Learning: The `FindComponentDependenciesLegacy` function in `internal/exec/stack_processor_utils.go` is legacy code that is not actively used and is kept only for backward compatibility purposes.
Applied to files:
internal/exec/component_resolver.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/component_resolver.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/component_resolver.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:
internal/exec/component_resolver.go
📚 Learning: 2024-10-20T00:41:57.135Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:93-98
Timestamp: 2024-10-20T00:41:57.135Z
Learning: When downloading schema files in `internal/exec/validate_stacks.go`, use a consistent temporary file name to overwrite the file each time and avoid creating multiple temporary files.
Applied to files:
internal/exec/component_resolver.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/component_resolver.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
internal/exec/component_resolver.go
📚 Learning: 2025-04-26T15:54:10.506Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 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/component_resolver.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/component_resolver.gocmd/describe_component.go
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
internal/exec/component_resolver.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:
cmd/describe_component.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:
cmd/describe_component.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
cmd/describe_component.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:
cmd/describe_component.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-11-08T19:56:18.647Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.647Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-09-07T18:07:00.549Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/describe_component.go
🧬 Code graph analysis (2)
internal/exec/component_resolver.go (6)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)pkg/logger/log.go (1)
Debug(24-26)pkg/utils/component_reverse_path_utils.go (1)
ExtractComponentInfoFromPath(30-62)errors/errors.go (3)
ErrPathResolutionFailed(464-464)ErrComponentTypeMismatch(462-462)ErrComponentNotInStack(463-463)internal/exec/utils.go (1)
FindStacksMap(245-295)
cmd/describe_component.go (2)
pkg/component/path.go (1)
IsExplicitComponentPath(17-22)internal/exec/component_resolver.go (1)
ResolveComponentFromPathWithoutTypeCheck(108-146)
⏰ 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). (9)
- GitHub Check: Build (windows)
- GitHub Check: Build (macos)
- GitHub Check: website-deploy-preview
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Review Dependency Licenses
- GitHub Check: autofix
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (1)
go.mod (1)
44-44: The mapstructure upgrade is properly executed and safe.The v2.4.0 release is stable, and the only intentional breaking change is the import path (from
github.com/mitchellh/mapstructuretogithub.com/go-viper/mapstructure/v2), while the API remains compatible. Verification across the codebase confirms all imports have been updated to the new path—no old references remain—with the previous package now appearing only as an indirect dependency.
Moved component resolution logic to pkg/component using DI pattern: - Created pkg/component/resolver.go with StackLoader interface - Implemented ExecStackLoader in internal/exec/stack_loader.go - Updated internal/exec/component_resolver.go to delegate to pkg/component - No circular dependencies: pkg/component → pkg/* (no internal/exec dependency) Benefits: - Clean architecture: business logic in pkg, orchestration in internal - Testable: StackLoader interface can be mocked - Backwards compatible: existing callers unchanged - Reusable: pkg/component can be used independently See ARCHITECTURE_REFACTOR.md for detailed design documentation.
Restored pkg/stack/stack_processor.go and pkg/stack/stack_processor_test.go which are required by pkg/spacelift integration. These files contain backwards-compatible wrapper functions that delegate to internal/exec. They were mistakenly deleted in the previous commit but are essential for Spacelift functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…Path
Added support for Windows-style relative paths (".\" and "..\") alongside
existing Unix-style paths ("./" and "../").
This ensures Windows users can use relative paths for component resolution
instead of falling back to component name lookup.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Changed: - /cli/commands/describe/describe-component → /cli/commands/describe/component - /cli/commands/validate/validate-component → /cli/commands/validate/component These URLs match the actual documentation structure where files are named describe-component.mdx but linked without the command prefix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
## Problem When ATMOS_BASE_PATH was set to a relative path (e.g., "../../.."), Atmos would resolve it relative to the current working directory (CWD) instead of relative to where atmos.yaml is located. This caused path resolution to fail when running Atmos commands from component subdirectories. ## Solution Created resolveAbsolutePath() helper function that resolves paths relative to atmosConfig.CliConfigPath (the directory containing atmos.yaml) when the path is relative. This allows path-based component resolution to work correctly regardless of CWD. Key changes: - Added resolveAbsolutePath() to resolve paths relative to CLI config path - Updated AtmosConfigAbsolutePaths() to resolve base path first, then join with component/stack paths - Fixed test configurations to use ATMOS_BASE_PATH (not ATMOS_BASE_DIR) - Added test snapshots for path-based component resolution tests ## Testing Verified that ATMOS_BASE_PATH="../../.." now correctly resolves paths when running from component directories. Related: Component path resolution feature (component-path-resolution branch)
Now that base_path resolution works correctly (resolving relative to atmos.yaml location instead of CWD), tests no longer need to override ATMOS_BASE_PATH. They can rely on base_path: "." from atmos.yaml. This proves the fix is working - tests now successfully: ✅ Find atmos.yaml via ATMOS_CLI_CONFIG_PATH ✅ Resolve base_path: "." correctly relative to atmos.yaml ✅ Load stack configurations Tests still fail on component resolution (feature incomplete), but path resolution infrastructure is now solid.
- Add ErrAmbiguousComponentPath error constant for clearer error messages - Update validateComponentInStack() to collect all matches and detect when multiple components reference the same directory - Return helpful error instructing users to use exact component name when path is ambiguous - Document limitation in cmd_utils.go where multiple components sharing a directory require exact names - Keep fast path without validation for performance (MVP approach) This enhancement handles the edge case where multiple components (e.g., "vpc/1", "vpc/2") both reference the same physical directory via metadata.component field. When a user runs from that directory, they now get a clear error message listing the conflicting components and instructing them to use the exact component name instead of the path. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Resolved conflict in cmd/cmd_utils_test.go by keeping both: - TestFilterChdirArgs from main - All path resolution and component tests from this branch
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 (2)
cmd/cmd_utils.go (2)
943-1000: Tighten cross‑platform path detection in completions and fix small comment nits.The new path-aware completion behavior is great, but there’s a small cross‑platform edge case and a tiny style nit:
- In
stackFlagCompletion, path detection currently treats only"."orfilepath.Separatoras a path:if component == "." || strings.Contains(component, string(filepath.Separator)) {On Windows, users often type Unix‑style paths like
./vpcorcomponents/terraform/vpc. Those contain'/'but notfilepath.Separator('\\'), so they’ll be treated as literal component names and skip resolution, even thoughComponentsArgCompletionhandles both separators.You can mirror the
ComponentsArgCompletionlogic here:- if component == "." || strings.Contains(component, string(filepath.Separator)) { + if component == "." || + strings.Contains(component, string(filepath.Separator)) || + strings.Contains(component, "/") {
- Minor: the new comments around the path-handling branches (both in
stackFlagCompletionandComponentsArgCompletion) don’t end with periods, which will annoygodot. Adding trailing periods keeps them consistent with the rest of the file and the project guidelines.Also applies to: 1142-1152
797-855: Use ErrorBuilder withWithCause()instead of manualfmt.Errorfwrapping for consistent error handling.The error construction in the path-resolution block mixes manual wrapping with ErrorBuilder, making it harder to reliably check
errors.Is(err, errUtils.ErrPathResolutionFailed).For the
InitCliConfig(info, false)failure:return schema.ConfigAndStacksInfo{}, errUtils.Build(errUtils.ErrPathResolutionFailed). WithCause(err). Err()For the
ResolveComponentFromPathWithoutValidationfailure, remove the intermediatewrappedErrand build directly from the sentinel:return schema.ConfigAndStacksInfo{}, errUtils.Build(errUtils.ErrPathResolutionFailed). WithCause(err). WithHint("Make sure the path is within your component directories"). Err()This keeps
ErrPathResolutionFailedas the sentinel while preserving the concrete cause, aligning with the established pattern in the codebase.All call sites pass valid component types (
terraform,helmfile,packer) and require no changes.
🧹 Nitpick comments (4)
errors/errors.go (1)
55-56: New sentinel errors align well with path-based resolution and validation flows.Adding explicit sentinels for missing Atmos config, non‑git contexts, invalid stack configuration, and the various path‑resolution cases (path not in component dirs, type mismatches, ambiguous paths, component‑not‑in‑stack, etc.) fits the centralized error pattern and supports the new resolver/tests that use
errors.Is. Just be mindful to keepErrInvalidStackConfigvsErrInvalidStackConfigurationusage clearly separated so callers don’t conflate the two.Also applies to: 79-84, 597-604
internal/exec/utils.go (1)
521-547: Consider propagating more path resolver sentinels instead of falling back to a generic component error.Right now you only special‑case
ErrPathNotInComponentDir; otherpathErrvalues fromResolveComponentFromPathWithoutValidation(e.g.,ErrComponentTypeMismatch,ErrAmbiguousComponentPath,ErrComponentNotInStack,ErrPathIsComponentBase) are effectively ignored and the user eventually sees a generic “invalid component / could not find component in stack” error.Forwarding those sentinel errors when present would surface the more specific diagnostics you’ve added in
errors/errors.goand keep behavior consistent with other call sites that rely onerrors.Isagainst these values. For example:} else { // For other resolver sentinels, return the path error directly. return configAndStacksInfo, pathErr }This would still preserve the fallback flow while giving callers the richer error information to format user‑facing messages.
cmd/cmd_utils.go (2)
585-679: Preserve underlying causes in new config validation errors.The new helpers look good and make config checks testable, but you’re dropping the original cause in a couple of places:
- In
validateAtmosConfig, whenu.IsDirectoryreturns an error, you buildErrStacksDirectoryDoesNotExistwithout attaching the underlyingerr, so diagnostics lose why the check failed (permissions vs. ENOENT, etc.).- In
checkAtmosConfigE, you ignoreerrfromvalidateAtmosConfigwhen buildingErrMissingAtmosConfig, so callers/telemetry can’t see whether the failure was due to malformedatmos.yaml, bad base path, missing stacks dir, etc.Consider wiring the cause through the builder while keeping the same user-facing hints, e.g.:
func checkAtmosConfigE(opts ...AtmosValidateOption) error { - err := validateAtmosConfig(opts...) - if err == nil { + err := validateAtmosConfig(opts...) + if err == nil { return nil } @@ - builder := errUtils.Build(errUtils.ErrMissingAtmosConfig). - WithExitCode(1) + builder := errUtils.Build(errUtils.ErrMissingAtmosConfig). + WithExitCode(1). + WithCause(err)Similarly, you could attach the
IsDirectoryerror when it’s non‑nil so debugging and tests can still see the root failure instead of just the sentinel.
1002-1019: Add documentation about intentional "terraform" default behavior.
determineComponentTypeFromCommandalready handles all three component types (terraform, helmfile, packer) correctly. The fallback to"terraform"when no known parent is found is intentional by design—visible in the test suite ("unknown command defaults to terraform", "no parent command defaults to terraform")—and safe in the calling context (stackFlagCompletionhas graceful error handling).Improve the doc comment to explicitly state: "If no terraform, helmfile, or packer ancestor is found in the command chain, defaults to 'terraform'." This clarifies the design intent for future maintainers without changing behavior.
Do not return
""instead; the terraform default is intentional and the tests explicitly verify this expected behavior.
📜 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 (10)
NOTICE(4 hunks)cmd/cmd_utils.go(8 hunks)cmd/cmd_utils_test.go(2 hunks)cmd/root.go(2 hunks)errors/errors.go(3 hunks)go.mod(5 hunks)internal/exec/utils.go(4 hunks)pkg/schema/schema.go(3 hunks)tests/cli_test.go(8 hunks)tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- cmd/root.go
- pkg/schema/schema.go
- tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden
- NOTICE
🧰 Additional context used
📓 Path-based instructions (7)
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
cmd/**/*.go: CLI command files must be thin wrappers that only handle CLI concerns (flags, arguments, command registration). All business logic must be delegated topkg/packages
Useflags.NewStandardParser()for command-specific flag parsing. NEVER callviper.BindEnv()orviper.BindPFlag()directly - use the unifiedpkg/flags/infrastructure instead. Register flags with Cobra command and bind to Viper for precedence handling
All CLI commands MUST implement the CommandProvider interface and register via command registry pattern incmd/internal/registry.go
Embed examples fromcmd/markdown/*_usage.mdusing//go:embed. Render withutils.PrintfMarkdown()
Files:
cmd/cmd_utils_test.gocmd/cmd_utils.go
**/*.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)
Import organization: three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages. Maintain aliases:cfg,log,u,errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions. Usenilif no atmosConfig param
Keep files small and focused (<600 lines). One cmd/impl per file. Co-locate tests. Never use//revive:disable:file-length-limit
Use sentinel errors fromerrors/errors.gowith ErrorBuilder for all user-facing errors. Useerrors.Is()for checking errors, never string comparison.WithCause()preserves underlying error messages,WithExplanation()adds context,WithHint()provides guidance,WithContext()adds key-value pairs
Usecontext.Contextas first parameter for: cancellation signals (prop...
Files:
cmd/cmd_utils_test.gointernal/exec/utils.gotests/cli_test.gocmd/cmd_utils.goerrors/errors.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 fixtures
**/*_test.go: Useassert.ErrorIs(t, err, ErrSentinel)for checking our own errors and stdlib errors. Only use string matching for third-party errors
NEVER useassert.Contains(err.Error(), ...)for error checking. ALWAYS useassert.ErrorIs()instead. NEVER use string comparison:err.Error() == "..."orstrings.Contains(err.Error(), ...)
Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests. Make code testable using dependency injection to avoidos.Exit,CheckErrorPrintAndExit, external systems. Remove always-skipped tests - fix or delete. Table-driven tests must have real scenarios
Usego.uber.org/mock/mockgenwith//go:generatedirectives for mock generation. Never create manual mocks. Pattern://go:generate go run go.uber.org/mock/mockgen@latest -source=filename.go -destination=mock_filename_test.go
Use table-driven tests for comprehensive coverage. Tests must call actual production code, never duplicate logic. Target >80% test coverage (enforced by CodeCov)
Files:
cmd/cmd_utils_test.gotests/cli_test.go
cmd/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS use
cmd.NewTestKit(t)for cmd tests. This auto-cleans RootCmd state (flags, args) and is required for any test touching RootCmd
Files:
cmd/cmd_utils_test.go
tests/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use
cmd.NewTestKit(t)for any test touching RootCmd. This auto-cleans RootCmd state. Tests skip gracefully with helpers fromtests/test_preconditions.go
Files:
tests/cli_test.go
tests/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use pipe redirection (
2>&1,| head,| tail) when running tests. Piping breaks TTY detection → ASCII fallback → wrong snapshots. NEVER manually edit golden snapshot files - always use-regenerate-snapshotsflag. Snapshots capture exact output including invisible formatting (lipgloss padding, ANSI codes, trailing whitespace)
Files:
tests/cli_test.go
{go.mod,go.sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Files:
go.mod
🧠 Learnings (82)
📓 Common learnings
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
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: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/path_utils.go:145-146
Timestamp: 2024-10-23T22:11:41.077Z
Learning: In the `atmos` project, the preference is to print relative paths in log messages instead of full paths.
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: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
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.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/cmd_utils.go:121-148
Timestamp: 2025-02-03T05:57:18.407Z
Learning: The Atmos CLI should fail fast (exit) when encountering configuration errors, including command alias configuration issues, to prevent undefined behavior. Use LogErrorAndExit instead of returning errors in such cases.
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.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
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.
📚 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:
cmd/cmd_utils_test.gointernal/exec/utils.gocmd/cmd_utils.gogo.moderrors/errors.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/cmd_utils_test.gotests/cli_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/cmd_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 cmd/**/*.go : Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Applied to files:
cmd/cmd_utils_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to pkg/**/*.go : Create focused purpose-built packages in `pkg/` for new functionality, each with clear responsibility. DO NOT add new functions to `pkg/utils/` or `internal/exec/`. Each package should have focused business logic with tests
Applied to files:
cmd/cmd_utils_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*_test.go : Use table-driven tests for comprehensive coverage. Tests must call actual production code, never duplicate logic. Target >80% test coverage (enforced by CodeCov)
Applied to files:
cmd/cmd_utils_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to tests/**/*.go : Use `cmd.NewTestKit(t)` for any test touching RootCmd. This auto-cleans RootCmd state. Tests skip gracefully with helpers from `tests/test_preconditions.go`
Applied to files:
cmd/cmd_utils_test.gotests/cli_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : New configs support Go templating with `FuncMap()` from `internal/exec/template_funcs.go`. Implement template functions in `internal/exec/template_funcs.go`, register them, add tests, and update documentation
Applied to files:
cmd/cmd_utils_test.gointernal/exec/utils.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests. Make code testable using dependency injection to avoid `os.Exit`, `CheckErrorPrintAndExit`, external systems. Remove always-skipped tests - fix or delete. Table-driven tests must have real scenarios
Applied to files:
cmd/cmd_utils_test.gotests/cli_test.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/cmd_utils_test.gointernal/exec/utils.gocmd/cmd_utils.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: The user confirmed that the errors package has an error string wrapping format, contradicting the previous learning about ErrWrappingFormat being invalid. The current usage of fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) appears to be the correct pattern.
Applied to files:
cmd/cmd_utils_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*_test.go : NEVER use `assert.Contains(err.Error(), ...)` for error checking. ALWAYS use `assert.ErrorIs()` instead. NEVER use string comparison: `err.Error() == "..."` or `strings.Contains(err.Error(), ...)`
Applied to files:
cmd/cmd_utils_test.gotests/cli_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*_test.go : Use `assert.ErrorIs(t, err, ErrSentinel)` for checking our own errors and stdlib errors. Only use string matching for third-party errors
Applied to files:
cmd/cmd_utils_test.gotests/cli_test.goerrors/errors.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/cmd_utils_test.gointernal/exec/utils.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
cmd/cmd_utils_test.gocmd/cmd_utils.gogo.moderrors/errors.go
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
cmd/cmd_utils_test.gocmd/cmd_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:
cmd/cmd_utils_test.gointernal/exec/utils.go
📚 Learning: 2025-01-07T20:38:09.618Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 896
File: cmd/editor_config.go:37-40
Timestamp: 2025-01-07T20:38:09.618Z
Learning: Error handling suggestion for `cmd.Help()` in `cmd/editor_config.go` was deferred as the code is planned for future modifications.
Applied to files:
cmd/cmd_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:
cmd/cmd_utils_test.gocmd/cmd_utils.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 cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
Applied to files:
cmd/cmd_utils_test.gocmd/cmd_utils.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/cmd_utils_test.gointernal/exec/utils.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : Import organization: three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages. Maintain aliases: `cfg`, `log`, `u`, `errUtils`
Applied to files:
cmd/cmd_utils_test.gointernal/exec/utils.gogo.moderrors/errors.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/cmd_utils_test.gocmd/cmd_utils.goerrors/errors.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
cmd/cmd_utils_test.gocmd/cmd_utils.gogo.moderrors/errors.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:
cmd/cmd_utils_test.gocmd/cmd_utils.goerrors/errors.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/cmd_utils_test.gointernal/exec/utils.gocmd/cmd_utils.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
cmd/cmd_utils_test.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:
cmd/cmd_utils_test.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
cmd/cmd_utils_test.gocmd/cmd_utils.goerrors/errors.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
cmd/cmd_utils_test.gointernal/exec/utils.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests. This is implemented through custom testing framework extensions and is used consistently throughout the test suite.
Applied to files:
cmd/cmd_utils_test.go
📚 Learning: 2025-10-08T06:48:07.499Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1602
File: internal/exec/stack_processor_utils.go:968-1003
Timestamp: 2025-10-08T06:48:07.499Z
Learning: The `FindComponentDependenciesLegacy` function in `internal/exec/stack_processor_utils.go` is legacy code that is not actively used and is kept only for backward compatibility purposes.
Applied to files:
internal/exec/utils.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions. Use `nil` if no atmosConfig param
Applied to files:
internal/exec/utils.go
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
internal/exec/utils.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
internal/exec/utils.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
internal/exec/utils.gotests/cli_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : Cross-platform compatible code (Linux/macOS/Windows): Use SDKs over binaries. Use `filepath.Join()`, not hardcoded path separators
Applied to files:
internal/exec/utils.gotests/cli_test.go
📚 Learning: 2024-12-25T20:28:19.618Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/workflow_utils.go:167-169
Timestamp: 2024-12-25T20:28:19.618Z
Learning: The user plans to revert the change from `path.Join` to `filepath.Join` in this PR due to testing gaps and will open a new PR to safely handle the migration without breaking `main`.
Applied to files:
internal/exec/utils.gotests/cli_test.go
📚 Learning: 2025-02-05T11:10:51.031Z
Learnt from: mss
Repo: cloudposse/atmos PR: 1024
File: internal/exec/go_getter_utils.go:31-33
Timestamp: 2025-02-05T11:10:51.031Z
Learning: The path traversal check in `ValidateURI` function in `internal/exec/go_getter_utils.go` is intentionally kept despite potentially blocking valid Git URLs, as this validation is planned to be addressed in a separate ticket.
Applied to files:
internal/exec/utils.gotests/cli_test.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/utils.gotests/cli_test.go
📚 Learning: 2024-11-22T12:38:33.132Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Applied to files:
internal/exec/utils.go
📚 Learning: 2025-02-03T15:51:48.035Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 984
File: internal/exec/go_getter_utils.go:103-109
Timestamp: 2025-02-03T15:51:48.035Z
Learning: When checking for subdirectories in GitHub URLs, use `parsedURL.Path` to check for "//" instead of the entire URL, as the scheme portion (e.g., "https://") will always contain "//".
Applied to files:
internal/exec/utils.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to pkg/io/**/*.go : Use I/O layer (`pkg/io/`) for stream access (stdout/stderr/stdin), terminal capabilities, masking. Use UI layer (`pkg/ui/`) for formatting (colors, styles, markdown rendering). Never use `fmt.Fprintf(os.Stdout/Stderr, ...)`, `fmt.Println()`, or direct stream access
Applied to files:
internal/exec/utils.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : Never use `fmt.Fprintf(os.Stdout/Stderr, ...)`, `fmt.Println()`, or direct stream access for I/O. Always use `data.*` for pipeable output or `ui.*` for user-facing messages
Applied to files:
internal/exec/utils.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to tests/**/*_test.go : NEVER use pipe redirection (`2>&1`, `| head`, `| tail`) when running tests. Piping breaks TTY detection → ASCII fallback → wrong snapshots. NEVER manually edit golden snapshot files - always use `-regenerate-snapshots` flag. Snapshots capture exact output including invisible formatting (lipgloss padding, ANSI codes, trailing whitespace)
Applied to files:
tests/cli_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: Ensure all tests pass, verify code coverage meets targets, run golangci-lint and fix any issues, and update documentation before submitting pull requests
Applied to files:
tests/cli_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Use dependency injection and interface-driven design to make code testable - avoid `os.Exit`, `CheckErrorPrintAndExit`, and direct external system calls in testable functions
Applied to files:
tests/cli_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to cmd/**/*_test.go : ALWAYS use `cmd.NewTestKit(t)` for cmd tests. This auto-cleans RootCmd state (flags, args) and is required for any test touching RootCmd
Applied to files:
tests/cli_test.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : For output: Pipeable data → `data.*` (stdout), Plain UI → `ui.Write*()` (stderr), Status messages → `ui.Success/Error/Warning/Info()` (stderr), Formatted docs → `ui.Markdown*()`. Use `data.Write()`, `data.Writef()`, `data.Writeln()`, `data.WriteJSON()`, `data.WriteYAML()` for data output
Applied to files:
tests/cli_test.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
cmd/cmd_utils.goerrors/errors.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
cmd/cmd_utils.gogo.mod
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
cmd/cmd_utils.gogo.moderrors/errors.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:
cmd/cmd_utils.go
📚 Learning: 2025-02-03T05:57:18.407Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/cmd_utils.go:121-148
Timestamp: 2025-02-03T05:57:18.407Z
Learning: The Atmos CLI should fail fast (exit) when encountering configuration errors, including command alias configuration issues, to prevent undefined behavior. Use LogErrorAndExit instead of returning errors in such cases.
Applied to files:
cmd/cmd_utils.go
📚 Learning: 2024-12-05T22:33:54.807Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_stacks.go:55-56
Timestamp: 2024-12-05T22:33:54.807Z
Learning: In the atmos project, the `u.LogErrorAndExit` function logs the error and exits the command execution appropriately within flag completion functions.
Applied to files:
cmd/cmd_utils.goerrors/errors.go
📚 Learning: 2025-01-18T15:15:41.645Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.
Applied to files:
cmd/cmd_utils.goerrors/errors.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
cmd/cmd_utils.go
📚 Learning: 2025-02-03T06:00:11.419Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.
Applied to files:
cmd/cmd_utils.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to cmd/**/*.go : Embed examples from `cmd/markdown/*_usage.md` using `//go:embed`. Render with `utils.PrintfMarkdown()`
Applied to files:
cmd/cmd_utils.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
cmd/cmd_utils.goerrors/errors.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/cmd_utils.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/cmd_utils.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/cmd_utils.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 never returns an error as it simply stores the completion function in an internal map. Error handling for this method call is unnecessary.
Applied to files:
cmd/cmd_utils.go
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1261
File: internal/exec/utils.go:639-640
Timestamp: 2025-05-22T15:42:10.906Z
Learning: In the Atmos codebase, when appending slices with `args := append(configAndStacksInfo.CliArgs, configAndStacksInfo.AdditionalArgsAndFlags...)`, it's intentional that the result is not stored back in the original slice. This pattern is used when the merged result serves a different purpose than the original slices, such as when creating a filtered version for component section assignments.
Applied to files:
cmd/cmd_utils.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 {go.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: Go version 1.23.0 was deliberately introduced by the maintainer (aknysh) in January 2025. While this might be a pre-release or development version of Go, it has been approved for use in this project.
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Applied to files:
go.mod
📚 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:
go.moderrors/errors.go
📚 Learning: 2025-02-14T16:16:36.328Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 1034
File: go.mod:3-3
Timestamp: 2025-02-14T16:16:36.328Z
Learning: Go 1.23.5 was released on January 16, 2025, and is a valid Go version. Always verify Go version information with official sources before suggesting version changes.
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:18:57.769Z
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.
Applied to files:
go.mod
📚 Learning: 2025-10-11T04:03:41.651Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1611
File: go.mod:3-3
Timestamp: 2025-10-11T04:03:41.651Z
Learning: The Go module "go" directive supports three-part version numbers (e.g., "go 1.24.8") since Go 1.21+. The format "go 1.N.P" is valid and recommended for specifying patch versions. Earlier Go versions only accepted major.minor format, but modern tooling fully supports the patch component.
Applied to files:
go.mod
📚 Learning: 2025-05-30T03:21:37.197Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.
Applied to files:
go.mod
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
go.mod
📚 Learning: 2025-11-10T23:23:39.771Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/registry/aqua/aqua_test.go:417-442
Timestamp: 2025-11-10T23:23:39.771Z
Learning: In Atmos toolchain AquaRegistry, tests should not hit real GitHub. Use the options pattern via WithGitHubBaseURL to inject an httptest server URL and make GetLatestVersion/GetAvailableVersions deterministic.
Applied to files:
go.mod
📚 Learning: 2025-10-02T19:17:51.630Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1504
File: pkg/profiler/profiler.go:20-31
Timestamp: 2025-10-02T19:17:51.630Z
Learning: In pkg/profiler/profiler.go, profiler-specific errors (ErrUnsupportedProfileType, ErrStartCPUProfile, ErrStartTraceProfile, ErrCreateProfileFile) must remain local and cannot be moved to errors/errors.go due to an import cycle: pkg/profiler → errors → pkg/schema → pkg/profiler. This is a valid exception to the centralized errors policy.
Applied to files:
go.mod
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : Use sentinel errors from `errors/errors.go` with ErrorBuilder for all user-facing errors. Use `errors.Is()` for checking errors, never string comparison. `WithCause()` preserves underlying error messages, `WithExplanation()` adds context, `WithHint()` provides guidance, `WithContext()` adds key-value pairs
Applied to files:
errors/errors.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
errors/errors.go
📚 Learning: 2024-12-05T02:48:53.818Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 809
File: cmd/cmd_utils.go:470-477
Timestamp: 2024-12-05T02:48:53.818Z
Learning: The function `GetLatestGitHubRepoRelease` in the Go codebase already uses a context with a timeout, so wrapping it with an additional context is unnecessary.
Applied to files:
errors/errors.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
errors/errors.go
📚 Learning: 2025-04-26T15:54:10.506Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 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:
errors/errors.go
🧬 Code graph analysis (3)
cmd/cmd_utils_test.go (4)
cmd/cmd_utils.go (2)
Contains(1176-1183)ComponentsArgCompletion(1142-1173)pkg/schema/schema.go (1)
ConfigAndStacksInfo(642-737)errors/errors.go (2)
ErrStacksDirectoryDoesNotExist(54-54)ErrNotInGitRepository(56-56)cmd/testkit_test.go (1)
NewTestKit(55-65)
internal/exec/utils.go (5)
pkg/schema/schema.go (4)
AtmosConfiguration(53-96)ConfigAndStacksInfo(642-737)Stacks(441-448)Context(500-515)pkg/config/utils.go (2)
GetContextFromVars(724-752)GetContextPrefix(755-825)internal/exec/stack_utils.go (1)
GetStackNamePattern(202-204)errors/errors.go (2)
ErrMissingStackNameTemplateAndPattern(52-52)ErrPathNotInComponentDir(598-598)internal/exec/component_resolver.go (1)
ResolveComponentFromPathWithoutValidation(44-52)
cmd/cmd_utils.go (4)
errors/errors.go (4)
ErrStacksDirectoryDoesNotExist(54-54)ErrMissingAtmosConfig(55-55)ErrPathResolutionFailed(601-601)ErrNotInGitRepository(56-56)pkg/schema/schema.go (2)
Stacks(441-448)ConfigAndStacksInfo(642-737)pkg/config/config.go (1)
InitCliConfig(28-67)internal/exec/component_resolver.go (2)
ResolveComponentFromPathWithoutValidation(44-52)ResolveComponentFromPath(18-27)
⏰ 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
|
@coderabbitai full review please |
|
@aknysh, I'll conduct another comprehensive review of this PR, focusing on aspects I haven't deeply covered before. 🔍 Deep Dive ReviewArchitecture & Design Decisions1. Package Organization
This avoids circular dependencies while maintaining clean interfaces. 2. Error Handling Strategy
3. Interface Design type StackLoader interface {
FindStacksMap(atmosConfig *schema.AtmosConfiguration, ignoreMissingFiles bool) (map[string]any, map[string]map[string]any, error)
}This interface enables dependency injection and testing. The Security & Robustness1. Path Traversal Protection
2. Edge Case Handling
Production Readiness1. Error Messages & UX ErrPathNotInComponentDir - Lists all configured component base paths
ErrAmbiguousComponentPath - Shows all matching components with command examples
ErrComponentNotInStack - Suggests running describe stacks to verify2. Observability
3. Backward Compatibility
Testing StrategyCoverage Analysis:
Test Quality:
Code Quality Observations1. Function Complexity
2. Platform Compatibility
3. Configuration Discovery Integration Points1. Command Integration
2. Tab Completion
DocumentationCompleteness:
Minor Observations
🎯 Final AssessmentStrengths
Areas for Future Enhancement (Optional)
Production Readiness: ✅ READYThis PR represents a well-engineered feature addition. The implementation demonstrates:
The optional improvements I've identified (both in this review and the previous one) are truly optional - they would be nice-to-haves but don't block production deployment. The core functionality is solid, secure, and well-tested. Recommendation: This PR is ready to merge. The optional enhancements can be addressed in follow-up PRs if desired. 🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
cmd/helmfile_path_resolution_test.go (1)
13-16: Align comments with godot punctuation rules.There are still a couple of comment lines without terminal punctuation:
- Line 15:
// Note: Since we don't have helmfile fixtures with components, this test verifies the path resolution- Line 111:
// The command will fail because we're not in a helmfile component directory,To keep
godothappy and comments consistent, consider something like:-// Note: Since we don't have helmfile fixtures with components, this test verifies the path resolution -// code path executes without panicking, even when components don't exist. +// Note: Since we don't have helmfile fixtures with components, this test verifies the path resolution. +// The code path executes without panicking, even when components don't exist. -// The command will fail because we're not in a helmfile component directory, -// but it should fail gracefully with a meaningful error. +// The command will fail because we're not in a helmfile component directory. +// However, it should fail gracefully with a meaningful error.This keeps the text the same while satisfying the linter.
Also applies to: 111-113
pkg/utils/component_reverse_path_utils_test.go (1)
610-677: Cross-platform handling via skipWindows is acceptable.The
skipWindowsflag addresses the hardcoded Unix paths issue from the past review. This is a pragmatic solution. For future improvement, consider constructing platform-agnostic test paths usingfilepath.Joinwith temp directories, but skipping is fine for now.cmd/cmd_utils_test.go (1)
847-865: Nice improvement: error wrapping test now actually verifieserrors.IsThis test now does what the comment says: it asserts
errors.Is(err, errUtils.ErrStacksDirectoryDoesNotExist)instead of just checking that the message is non-empty. That directly exercises the error builder/wrapping behavior ingetConfigAndStacksInfo/validateAtmosConfigand addresses the earlier review concern.
🧹 Nitpick comments (32)
cmd/terraform_utils_test.go (1)
273-325: Edge-case matrix looks solid; tighten the table a bit.The additional combinations for
PlanFile/UseTerraformPlanwith multi-component flags give good coverage ofErrInvalidTerraformSingleComponentAndMultiComponentFlags, and usingNewTestKitplusassert.ErrorIsis exactly what we want for these sentinels.One small cleanup: every case currently sets a non‑nil
expectedError, so theelsebranch (assert.NoError) is dead. Either add at least one “valid” scenario to this table or drop theelseand always assert an error; right now it adds a bit of noise for future readers.tests/cli_validation_test.go (2)
74-82: Silent validators wired correctly; consider trimming unusederrorCheck.The switch to
validateYAMLFormatSilent,validateJSONFormatSilent, andvalidateFormatValidationSilentwith boolean checks keeps these table-driven tests focused on behavior and avoids noisytesting.Tcoupling; this looks solid.One minor nit: the
errorCheckfield in the YAML/JSON tables is no longer used anywhere, so it’s just config noise now. Either drop it or reintroduce explicit assertions around error text if you still want to validate messaging.Also applies to: 171-179, 234-241
281-289: Error‑context test names/comments no longer match what’s asserted.After switching to the
*Silenthelpers, these tests now only assert that validation fails, not that previews/offsets/truncation are emitted. If you still care about the error-context behavior, consider:
- Calling the non‑silent path (or a wrapper) here and asserting on captured output, or
- Renaming the tests and updating comments to describe the boolean semantics they actually cover.
As is, the behavior is fine, but the test intent is a bit misleading.
Also applies to: 292-300, 334-343
tests/cli_validation_edge_test.go (2)
263-287: Preview truncation test name vs assertions are a bit out of sync.This test is named and commented as validating preview truncation, but after switching to
validateYAMLFormatSilentit still only asserts that the input is valid YAML (no direct checks on preview length or line count). If preview behavior is important, consider a follow-up that asserts on the actual preview output (e.g., by factoring the preview logic into a helper that returns the truncated string) instead of relying solely on pass/fail.
319-327: JSON syntax error context test now depends on a “silent” helper.
validateJSONFormatSilentsuggests minimal/no test-output side effects, but the comment still says context reporting is done viat.Errorfinside the function. It’d be good to either:
- Clarify the comment if context logging is no longer part of this path, or
- Add a separate test that asserts on whatever structure/string now carries the context, if that behavior remains important.
cmd/describe_component_test.go (5)
30-56: Consider using ErrorIs instead of string matching.The TestKit migration is solid, but lines 53-54 use string-based error checking via
assert.NotContains. Per coding guidelines, preferassert.ErrorIs()for error type checking when specific error sentinels are available.
58-86: Consider using ErrorIs instead of string matching.Same recommendation as the previous test: lines 83-84 would benefit from
assert.ErrorIs()instead of string-based error checking when checking for specific error types.
88-134: Strengthen test assertions beyond "no panic" checks.This test primarily verifies that the command doesn't panic, with only a weak string-based error check at line 130. Consider adding assertions that validate actual path resolution behavior, such as verifying resolved component names or checking specific error types with
assert.ErrorIs().Based on PR context, similar weak assertions in helmfile and packer tests were flagged for strengthening.
136-172: Consider verifying specific error types.The test correctly validates that config load failures produce errors, but could be strengthened by using
assert.ErrorIs()to check for specific config-related error sentinels instead of just verifying that an error occurred.
174-200: Strengthen assertions to test actual auth manager behavior.This test primarily ensures auth manager creation doesn't panic, with a weak string-based check at line 198. Consider adding assertions that validate auth manager functionality or use
assert.ErrorIs()to check for specific auth-related error types.cmd/terraform_test.go (1)
3-14: Tests correctly adapt to terraformRun returning an error.Using
errUtils.GetExitCode(err)beforeos.Exitpreserves the intended behavior of asserting on the process exit code while aligning with the centralized exit-code logic.If you touch these again, you could factor the repeated
TEST_EXITharness (env setup +terraformRun+ exit-code handling) into a small helper to DRY upTestTerraformRun1/2/3, but it’s not critical.Also applies to: 30-35, 66-71, 103-107
tests/snapshots/TestCLICommands_helmfile_command_with_terraform_component_path.stderr.golden (1)
1-12: Path-based type-mismatch error output looks solid.The error clearly explains that the component type inferred from the path doesn’t match the
helmfilecommand, and the generic hint about component directories is still directionally useful.If you later refine the underlying error builder, consider adding a more specific hint for this case (e.g., suggesting a helmfile component path when running helmfile commands), then update this snapshot accordingly.
cmd/packer_path_resolution_test.go (1)
15-80: Consider strengthening test assertions.The tests currently verify "no panic" and check error types, but don't assert specific outcomes or messages. While this validates the path resolution code path executes, stronger assertions would improve confidence—for example, verifying specific error messages or checking that the resolved component name appears in error context.
Based on learnings, similar command-level tests (helmfile_path_resolution_test.go, packer_path_resolution_test.go) were flagged for having assertions that only verify "no panic."
If test fixtures reliably reproduce these scenarios, consider adding assertions like:
// Example: verify error message contains expected path assert.Contains(t, err.Error(), "components/packer")internal/exec/validate_component.go (1)
224-228: Consider extracting duplicatedbasePathToUselogic.The fallback pattern appears twice. A small helper could reduce repetition:
+func getBasePathToUse(atmosConfig *schema.AtmosConfiguration) string { + if atmosConfig.BasePathAbsolute != "" { + return atmosConfig.BasePathAbsolute + } + return atmosConfig.BasePath +}Then both usages become
basePathToUse := getBasePathToUse(atmosConfig).Also applies to: 268-272
pkg/config/load_test.go (1)
1029-1031: Minor: consider asserting config file path.When
expectConfigis true, you assertv.ConfigFileUsed()is not empty. Consider also verifying it points to the expected directory for stronger assertions:if tt.expectConfig { assert.NotEmpty(t, v.ConfigFileUsed()) + assert.Contains(t, v.ConfigFileUsed(), "atmos.yaml") }internal/exec/stack_loader_test.go (1)
44-67: Tighten the interface conformance check (comment vs behavior).The
TestExecStackLoader_ImplementsInterfacetest is effectively a runtime smoke test; the comment says “compile-time check”, which is a bit misleading. If you want a true compile-time assertion thatExecStackLoaderimplements the interface, consider the usual pattern:// var _ component.StackLoader = (*ExecStackLoader)(nil)in a non-test file (or this test) and adjust the comment accordingly. The current test is fine functionally; this is just a clarity/idiomaticity tweak.
pkg/component/resolver_internal_test.go (1)
94-107: Prefer sentinel-based error checks over string matching in these tests.The tables for
TestExtractComponentsSectionandTestHandleComponentMatchescurrently assert on substrings oferr.Error()(e.g.,assert.Contains(t, err.Error(), tt.errContains)). Given the repo rule to avoid string-based error checks, it would be more robust to:
- Expose or reuse sentinel errors from the implementation (via
errors.Is/assert.ErrorIs) for each scenario, and- Optionally keep a secondary
Contains/message check only for user-facing text if really needed.This will make the tests less brittle if error wording evolves while preserving strong behavioral guarantees.
Also applies to: 272-283
cmd/describe_component.go (1)
78-107: Path-based resolution wiring looks solid; consider preserving the init error as a cause.The flow of:
- Detecting explicit paths via
comp.IsExplicitComponentPath,- Enabling
processStacksonly when needed, and- Resolving to a component name with
ResolveComponentFromPathWithoutTypeCheckis a nice, minimal way to layer path support without regressing name-only behavior.
One small improvement: in the
InitCliConfigerror path whenneedsPathResolutionis true, you build anErrPathResolutionFailederror with hints and context, but the originalerris only captured as a string field:pathErr := errUtils.Build(errUtils.ErrPathResolutionFailed). WithHintf("Failed to initialize config for path: `%s`\n\nPath resolution requires valid Atmos configuration", component). ... WithContext("config_error", err.Error()). WithExitCode(2). Err()To keep
errors.Is/errors.Unwrapuseful and avoid losing the underlying sentinel (e.g.,ErrStackNotFound), it would be worth addingWithCause(err)to that builder chain. That way callers can still distinguish configuration vs. path issues while benefiting from the richer path-level context.Also applies to: 110-124
cmd/helmfile_path_resolution_test.go (1)
108-123: Strengthen the assertion in the “current directory” test.In
TestHelmfilePathResolutionWithCurrentDir, insideif err != nilyou assert:assert.True(t, isPathError || err != nil, ...)Given you’re already in the
err != nilbranch, this condition is always true, so the test only guards “no panic” and doesn’t actually validate that a path-related error was produced.If the intent is to confirm that
.goes through the path-resolution code path, it’d be more meaningful to assert just on the sentinel(s), e.g.:assert.True(t, isPathError, "Current directory resolution should produce a path resolution error, got: %v", err)or to assert
errors.Isagainst the specific sentinel you expect most often in this scenario.cmd/validate_component_test.go (1)
198-214: Avoid relying solely on string matching for the “missing stack” error.
TestResolvePathBasedComponent_MissingStackcurrently checks:assert.Error(tk, err) assert.Empty(tk, result) assert.Contains(tk, err.Error(), "--stack")Per repo guidelines, tests for our own errors should prefer
assert.ErrorIsover inspectingerr.Error(). IfresolvePathBasedComponentreturns a dedicated sentinel (e.g., a “missing stack for path resolution” error), consider:
- Asserting
assert.ErrorIs(tk, err, errUtils.SomeSentinel)first, and- Optionally keeping the
Containscheck only as a secondary assertion for the user-facing message if you really need to lock that in.That will make the test more resilient to future wording changes while still validating the behavior.
pkg/component/resolver_mock_test.go (1)
7-39: MockStackLoader is fine; consider mockgen for future extensibility.This hand-written
MockStackLoaderis small and clear, and it’s safely confined to a_test.gofile. Over time, if theStackLoaderinterface grows or tests start needing more nuanced behavior (e.g., per-call expectations), it may be worth switching to a generated mock viago.uber.org/mock/mockgento stay aligned with the repo’s “no manual mocks” preference and reduce maintenance overhead.cmd/terraform_utils.go (1)
61-75: Mixed error handling patterns.Lines 62, 65, 68, 71, 74 still use
CheckErrorPrintAndExitwhile the function signature returns an error. This works but creates inconsistency within the same function.Consider unifying to return errors for consistency:
- processTemplates, err := flags.GetBool("process-templates") - errUtils.CheckErrorPrintAndExit(err, "", "") + processTemplates, err := flags.GetBool("process-templates") + if err != nil { + return err + }This can be addressed in a follow-up if preferred.
pkg/config/config.go (1)
201-233: Clean path resolution logic with proper fallback.The three-step resolution order (absolute → relative to cliConfigPath → CWD fallback) handles the ATMOS_BASE_PATH relative path scenario correctly. Documentation is clear.
One note: the raw errors from
filepath.Absare returned without wrapping. Per coding guidelines, consider adding context:- return "", err + return "", fmt.Errorf("resolving path %q relative to config: %w", path, err)This would help users understand which path failed.
internal/exec/component_resolver_test.go (1)
15-82: Consider expanding test cases for better coverage.The table-driven structure is good, but there's only one test case that always expects an error. The
setupStacksandskipStackValidationfields are defined but not used. Either add more cases or remove unused fields.pkg/utils/component_reverse_path_utils.go (2)
1-13: Consider moving to a dedicated package.Based on coding guidelines, new functionality should go into focused purpose-built packages in
pkg/rather thanpkg/utils/. This component path resolution logic has a clear responsibility and could live inpkg/component/alongside the resolver.That said, the implementation is solid and this is non-blocking.
129-136: Silent error suppression in resolveBasePath.The error from
filepath.Absis discarded. While rare, this could mask filesystem issues.func resolveBasePath(basePath string) string { if basePath == "" || filepath.IsAbs(basePath) { return basePath } - absPath, _ := filepath.Abs(basePath) - return absPath + absPath, err := filepath.Abs(basePath) + if err != nil { + // Fallback to original path if resolution fails. + return basePath + } + return absPath }pkg/component/resolver_test.go (1)
18-31: Consider removing redundant error string check.Line 30 uses
assert.Contains(err.Error(), ...)after already usingassert.ErrorIs. TheErrorIscheck is sufficient for validating the error type. The string check is redundant since the error message content is implementation detail.assert.Error(t, err) assert.ErrorIs(t, err, errUtils.ErrComponentNotInStack) assert.Empty(t, result) - assert.Contains(t, err.Error(), "not found in stack")Same pattern appears at lines 78, 338, etc. Non-blocking but cleaner without.
cmd/cmd_utils_test.go (2)
804-817: Avoid string-based error assertions; preferassert.ErrorIsfor stack-dir sentinelHere you’re checking the error content via
assert.Contains(t, err.Error(), "directory for Atmos stacks does not exist", ...). Per the repo’s testing guidelines, error checks for our own sentinels should useassert.ErrorIs, not string inspection, to avoid brittleness if messages change.Since
validateAtmosConfigandgetConfigAndStacksInfonow surfaceerrUtils.ErrStacksDirectoryDoesNotExist, you can assert on that directly:- assert.Error(t, err, "Should return error for missing config instead of calling os.Exit()") - assert.Contains(t, err.Error(), "directory for Atmos stacks does not exist", "Error should relate to config validation") + assert.Error(t, err, "Should return error for missing config instead of calling os.Exit()") + assert.ErrorIs(t, err, errUtils.ErrStacksDirectoryDoesNotExist)This keeps the test aligned with the intended error-wrapping behavior. As per coding guidelines, string matching on
err.Error()should be reserved for truly third‑party errors.
1278-1285: Consider usingNewTestKit(t)when exercisingRootCmd-derived state
TestGetTopLevelCommandscallsgetTopLevelCommands(), which in turn depends onRootCmd.Commands(). The project’s test rules recommend usingcmd.NewTestKit(t)for any test that touchesRootCmdso command registration and global state are consistently initialized/cleaned.A small tweak like this would align the test with that pattern:
func TestGetTopLevelCommands(t *testing.T) { - result := getTopLevelCommands() + _ = NewTestKit(t) + + result := getTopLevelCommands()Not urgent, but it will make this test more robust against future changes to root command setup. Based on learnings, this follows the established
cmdtesting pattern.cmd/cmd_utils.go (2)
585-679: Config validation refactor is sound; minor opportunity to preserve original cause in wrapperThe new
validateAtmosConfig,checkAtmosConfig, andcheckAtmosConfigEsplit nicely between:
- A core validator that returns errors (testable, used by
getConfigAndStacksInfo), and- Legacy wrappers that preserve “exit on failure” behavior with richer messaging.
checkAtmosConfigEin particular builds a structuredErrMissingAtmosConfigwith explanation and hints, which is a good fit for user-facing errors.One minor refinement: when you already have a concrete validation error (e.g., from
validateAtmosConfig), you could consider preserving it as a cause inside the builder rather than only via textual context. For example, usingWithCause(err)on theErrMissingAtmosConfigbuilder would keeperrors.Isworking on the original sentinel while still layering the high-level “missing config” classification and explanation. That would make downstream tests or tooling that care about the precise root cause more reliable, while keeping the UX unchanged.Not a blocker; more of a future hardening step to align with the documented error-builder pattern.
829-880: TightenhandlePathResolutionErrorto use the sentinel directly withWithCauseThe overall structure in
resolveComponentPathandhandlePathResolutionErroris good: known, richly-wrapped resolver errors (ambiguous path, component not in stack, stack not found, user aborted) are passed through untouched, while everything else is normalized underErrPathResolutionFailedwith an extra hint.The implementation in
handlePathResolutionErrorcurrently does:wrappedErr := fmt.Errorf("%w: %w", errUtils.ErrPathResolutionFailed, err) return errUtils.Build(wrappedErr). WithHint("Make sure the path is within your component directories"). Err()Two minor issues with that pattern:
- It builds on a non-sentinel
wrappedErrinstead of the canonicalErrPathResolutionFailed, which drifts from the “builder around sentinel” convention.- It uses
fmt.Errorfas a first layer, rather than attaching the original error as a cause via the builder, which is what the error guidelines describe (WithCause()for preserving underlying errors).A more idiomatic version would be along the lines of:
return errUtils.Build(errUtils.ErrPathResolutionFailed). WithCause(err). WithHint("Make sure the path is within your component directories"). Err()This keeps
errors.Is(err, errUtils.ErrPathResolutionFailed)working reliably and preserves the underlying resolver error for debugging while still adding the hint.pkg/component/resolver.go (1)
231-270:loadStackConfigwraps stack-loading failures; consider preserving the underlying cause more structurally
loadStackConfigcorrectly:
- Delegates stack map loading to the injected
StackLoader,- Returns an
ErrStackNotFound-based error when loading fails or when the stack key is absent, and- Validates that the retrieved stack config is a
map[string]any.The error builder usage is mostly aligned with the guidelines (sentinel + hints + context). One small enhancement would be to attach the original
FindStacksMaperror as a cause rather than only including it as text and aunderlying_errorcontext value. For example:loadErr := errUtils.Build(errUtils.ErrStackNotFound). WithCause(err). WithHintf("Failed to load stack configurations: %s\n\nPath-based component resolution requires valid stack configuration", err.Error()). ... Err()That would let
errors.Is/errors.Asintrospect the underlying issue while keeping the user-facing sentinel and hints exactly as they are.
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cmd/describe_component.go (1)
24-123: Path-based component handling indescribe componentis wired correctly; one repeat comment nit.The updated
RunEflow looks good:
comp.IsExplicitComponentPathcleanly distinguishes path-like args (.,./,../, absolute) from logical component names (including ones with/), so existingcomponentsemantics stay intact.- Passing
needsPathResolutionintocfg.InitCliConfigensures stacks are processed only when required, so the resolver hasStackConfigFilesAbsolutePathsavailable without always paying that cost.- The
needsPathResolutionbranch resolving viae.ResolveComponentFromPathWithoutTypeCheckbefore auth creation is the right place to map a path into a component name and to surface detailed resolver errors.- When config init fails and we were attempting path resolution, the
ErrPathResolutionFailedbuilder with hints/context is a nice improvement over a bare config error.Minor
godotnit (this was flagged previously): in the new comment block aboveInitCliConfig, the first two lines are full sentences without trailing periods:
- Line 83:
// Load atmos configuration. Use processStacks=true when path resolution is needed- Line 84:
// because the resolver needs StackConfigFilesAbsolutePaths to find stacks andAdding periods there will keep comment style consistent and avoid linter noise.
cmd/helmfile_path_resolution_test.go (1)
13-124: Helmfile path-resolution smoke tests are on point; two lingering comment punctuation issues.These tests do a nice job of:
- Exercising shared path-resolution via
RootCmd.SetArgs/Executefor helmfile, both for plain component names and explicit path-like args (including".").- Skipping cleanly when
helmfileor the fixture tree is unavailable.- Asserting that path-based invocations fail with one of the expected path/stack/component sentinels rather than panicking or returning a nil/opaque error.
Two comment lines still lack trailing periods and will annoy
godot(this was mentioned in an earlier review):
- Line 15:
// Note: Since we don't have helmfile fixtures with components, this test verifies the path resolution- Line 111:
// The command will fail because we're not in a helmfile component directory,Adding periods there will make the linter happy.
🧹 Nitpick comments (6)
pkg/utils/component_reverse_path_utils.go (1)
15-344: Path→component inference logic looks correct; consider moving out ofpkg/utilsand fix minor comment nits.The overall flow in
ExtractComponentInfoFromPathand its helpers looks solid:
- Normalization (
normalizePathForResolution) handles., relative paths,filepath.Abs, and symlinks with good, hint-richErrPathResolutionFailederrors.validatePathIsNotConfigDirectorycorrectly reuses pre-resolved stacks/workflows base paths and guards against treating config dirs as components, returningErrPathNotInComponentDirwith helpful hints.tryExtractComponentType/validatePathWithinBaseenforce that the path lives under the resolved component base path and map it into{FolderPrefix, ComponentName, FullComponent}with OS-agnostic slashes.Two minor points:
- Repo guidance prefers purpose-built packages over
pkg/utils/for new functionality; this file is effectively a focused “component path inference” service and might be a better long‑term fit underpkg/component(next to the new path/resolver types) rather thanpkg/utils. Consider a future move when convenient.- A couple of doc/comment lines don’t end with a period and may upset
godot— for example the bullet comments underExtractComponentInfoFromPathand any multi-line comment where an intermediate line is a full sentence (e.g., if you add more such lines later). It’s minor, but worth keeping consistent with the rest of the repo.internal/exec/validate_component.go (1)
20-241: Correctly preferringBasePathAbsolutefor schema/module resolution; one small comment nit.Using
getBasePathToUseto preferatmosConfig.BasePathAbsolutewhen joiningschemaPathand OPAmodulePathsfixes the relative-base_pathissue and alignsvalidateComponentInternalwith how the rest of the config now tracks absolute paths. The fallback toBasePathpreserves backward compatibility, and the file existence checks remain the same.Minor style nit for the
godotlinter: the second line of the new comment abovegetBasePathToUse(line 21) ends with “relative” and no period. Making that a full sentence ending with a period will keep comment style consistent.internal/exec/stack_loader_test.go (1)
13-58: ExecStackLoader tests are appropriate smoke checks; tidy comment punctuation.The tests correctly:
- Assert
NewStackLoaderreturns a non-nil*ExecStackLoader.- Smoke-test
FindStacksMapwithignoreMissingFiles=trueto ensure the method can be called without panicking, regardless of missing fixtures.- Enforce compile-time interface conformance (
var _ component.StackLoader = (*ExecStackLoader)(nil)) and a simple runtime assignment.Two small comment nits for
godot:
- Line 28:
// This will likely fail due to missing files, but we're testing that→ add a period.- Line 51:
// The compile-time check above ensures interface conformance; this test verifies→ add a period.Otherwise this file looks good.
pkg/component/resolver_test.go (1)
1033-1040: UNC path panic check could be stronger.The assertion
assert.NotContains(t, err.Error(), "panic")only catches if "panic" appears in the error message. A true panic would terminate the test before reaching this assertion. Consider wrapping in a defer/recover if you want to catch actual panics.// Test with a UNC-style path - should fail gracefully, not panic. -_, err := resolver.ResolveComponentFromPath(atmosConfig, `\\server\share\project\components\terraform\vpc`, "dev", "terraform") -// Error is expected (path likely doesn't exist), but no panic should occur. -// The test verifies the code handles UNC paths without panicking. -if err != nil { - assert.NotContains(t, err.Error(), "panic") -} +// Error is expected (path likely doesn't exist), but no panic should occur. +require.NotPanics(t, func() { + _, _ = resolver.ResolveComponentFromPath(atmosConfig, `\\server\share\project\components\terraform\vpc`, "dev", "terraform") +})pkg/component/resolver.go (2)
272-311: Optional: Consider ErrorBuilder for user-facing errors.The function uses plain
fmt.Errorffor errors that bubble up to users (lines 281, 286, 292, 302). While functional, using ErrorBuilder would provide richer context and hints, matching the pattern used elsewhere in this file.Example for line 281:
- return nil, fmt.Errorf("%w: stack '%s' has no components section", errUtils.ErrComponentNotInStack, stack) + return nil, errUtils.Build(errUtils.ErrComponentNotInStack). + WithHintf("Stack '%s' has no components section", stack). + WithHint("Verify your stack configuration includes a components section"). + WithContext("stack", stack). + WithExitCode(2). + Err()
459-507: LGTM: Polished interactive selection.Good use of huh library with proper abort handling (lines 494-495), theme application (line 489), and keymap configuration (lines 477-481). Line 497 could optionally use ErrorBuilder for consistency, though it's an edge case error.
| // TestGetConfigAndStacksInfo tests the error-returning version of config and stacks info processing. | ||
| // This test was not possible before refactoring because getConfigAndStacksInfo() called os.Exit() on errors. | ||
| func TestGetConfigAndStacksInfo(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| commandName string | ||
| args []string | ||
| description string | ||
| }{ | ||
| { | ||
| name: "empty args", | ||
| commandName: "terraform", | ||
| args: []string{}, | ||
| description: "Should handle empty args without panicking or calling os.Exit()", | ||
| }, | ||
| { | ||
| name: "args with double dash", | ||
| commandName: "terraform", | ||
| args: []string{"plan", "--", "extra", "args"}, | ||
| description: "Should properly split args at double dash without panicking", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // Create a minimal cobra command for testing. | ||
| cmd := &cobra.Command{ | ||
| Use: tt.commandName, | ||
| } | ||
|
|
||
| // The key test: this call should return an error instead of calling os.Exit(). | ||
| info, err := getConfigAndStacksInfo(tt.commandName, cmd, tt.args) | ||
|
|
||
| // We expect an error here because we're not in a valid atmos directory, | ||
| // but the important thing is that it doesn't call os.Exit() | ||
| // and we can actually test the error behavior. | ||
| if err != nil { | ||
| // This is expected - we're testing that errors are returned properly. | ||
| t.Logf("Got expected error (no valid config): %v", err) | ||
| assert.Error(t, err, tt.description) | ||
| } else { | ||
| // If somehow we got a valid config, verify the info structure. | ||
| assert.IsType(t, schema.ConfigAndStacksInfo{}, info, "Should return ConfigAndStacksInfo struct") | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // TestGetConfigAndStacksInfoDoubleDashHandling tests that double-dash arguments are properly separated. | ||
| // This verifies the refactored function maintains the original behavior. | ||
| func TestGetConfigAndStacksInfoDoubleDashHandling(t *testing.T) { | ||
| cmd := &cobra.Command{ | ||
| Use: "terraform", | ||
| } | ||
|
|
||
| // Test with args containing "--". | ||
| args := []string{"plan", "component", "--stack", "dev", "--", "-target=resource"} | ||
|
|
||
| _, err := getConfigAndStacksInfo("terraform", cmd, args) | ||
|
|
||
| // We expect this to fail due to missing atmos config, but importantly | ||
| // it should NOT panic or call os.Exit(). | ||
| assert.Error(t, err, "Should return error for missing config instead of calling os.Exit()") | ||
| assert.ErrorIs(t, err, errUtils.ErrStacksDirectoryDoesNotExist) | ||
| } | ||
|
|
||
| // TestGetConfigAndStacksInfoReturnsErrorInsteadOfExiting demonstrates the key improvement. | ||
| // Before refactoring, this test would have been impossible because the function called os.Exit(). | ||
| func TestGetConfigAndStacksInfoReturnsErrorInsteadOfExiting(t *testing.T) { | ||
| cmd := &cobra.Command{ | ||
| Use: "terraform", | ||
| } | ||
|
|
||
| // Call the function with invalid input that would previously cause os.Exit(1). | ||
| _, err := getConfigAndStacksInfo("terraform", cmd, []string{}) | ||
|
|
||
| // The key assertion: we got an error back instead of the process terminating. | ||
| assert.Error(t, err, "Function should return error instead of calling os.Exit()") | ||
|
|
||
| // Verify it contains useful information. | ||
| assert.NotEmpty(t, err.Error(), "Error should have a descriptive message") | ||
| } | ||
|
|
||
| // TestValidateAtmosConfigWithOptions tests that options pattern works correctly. | ||
| func TestValidateAtmosConfigWithOptions(t *testing.T) { | ||
| // Test that WithStackValidation option is respected. | ||
| err := validateAtmosConfig(WithStackValidation(false)) | ||
| // With stack validation disabled, we should only fail on config loading. | ||
| // (which will fail in test env, but that's OK - we're testing it returns an error) | ||
| if err != nil { | ||
| t.Logf("Got expected error with stack validation disabled: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Good coverage for error-returning behavior; fix a few comment punctuation nits.
The new tests around getConfigAndStacksInfo and validateAtmosConfig nicely exercise empty-args, -- handling, and error wrapping (errors.Is against ErrStacksDirectoryDoesNotExist), while ensuring we no longer call os.Exit(). That behavior looks correct.
Minor lint nits: a few new comments don’t end with a period and will trip the godot linter, e.g.:
- Line 787:
// We expect an error here because we're not in a valid atmos directory, - Line 788:
// but the important thing is that it doesn't call os.Exit() - Line 813:
// We expect this to fail due to missing atmos config, but importantly - Line 841:
// (which will fail in test env, but that's OK - we're testing it returns an error)
Adding trailing periods (or reflowing into full-sentence lines) will keep lint happy.
🤖 Prompt for AI Agents
In cmd/cmd_utils_test.go around lines 753 to 845, several comment lines fail the
godot linter because they do not end with a period; update the comments on lines
787, 788, 813, and 841 to end with a period (or reflow them into full sentences)
so each comment is a proper sentence and satisfies the godot rule.
2 participants
Summary
Enables developers to use filesystem paths (e.g.,
.,./vpc) instead of component names with all component-related commands. Paths are automatically resolved to component names based on stack configuration, eliminating the need to memorize component names.Changes
describe component, andvalidate componentcommandsExample Usage
Features
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Other
✏️ Tip: You can customize this high-level summary in your review settings.