Add list affected command with spinner UI improvements #1874
Conversation
|
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 FilesNone |
📝 WalkthroughWalkthroughAdds a new atmos "list affected" command and wiring, introduces git worktree helpers, refactors spinner UI into pkg/ui/spinner, fixes describe-affected path/cache bugs, adds metadata/status_text, and includes tests and docs for the new command and UI changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User as CLI User
participant Cmd as cmd/list/affected
participant Exec as pkg/list.ExecuteListAffectedCmd
participant DA as internal/exec (DescribeAffected)
participant Git as pkg/git (worktree)
participant Render as pkg/list/renderer
participant UI as pkg/ui/spinner
User->>Cmd: atmos list affected --ref origin/main --sha abc123
Cmd->>Cmd: parse flags, validate config
Cmd->>Exec: ExecuteListAffectedCmd(opts)
Exec->>Exec: decide path (RepoPath / CloneTargetRef / default)
alt CloneTargetRef
Exec->>Git: CreateWorktree(repoDir, targetCommit)
Git-->>Exec: worktreePath
Exec->>DA: DescribeAffected(worktreePath)
DA-->>Exec: affected data
Exec->>Git: RemoveWorktree(repoDir, worktreePath)
else RepoPath provided
Exec->>DA: DescribeAffected(repoPath)
DA-->>Exec: affected data
else Default
Exec->>DA: DescribeAffected(local repo)
DA-->>Exec: affected data
end
Exec->>Exec: extract columns, apply filters & sorters
Exec->>Render: render output (table/json/yaml/csv/tsv/tree)
Render-->>User: formatted output
Note over UI: Spinner shown in TTY mode via pkg/ui/spinner during DescribeAffected and rendering
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 |
![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
🧹 Nitpick comments (9)
pkg/git/worktree.go (5)
30-30: Usefilepath.Joinfor cross-platform compatibility.String concatenation with
/may cause issues on Windows.+ "path/filepath" "strings"- worktreePath := tempParentDir + "/worktree" + worktreePath := filepath.Join(tempParentDir, worktreeSubdir)
22-54: Add performance tracking for public function.Per coding guidelines, public functions should include
defer perf.Track(nil, "git.CreateWorktree")().+ "github.com/cloudposse/atmos/pkg/perf"func CreateWorktree(repoDir, targetCommit string) (string, error) { + defer perf.Track(nil, "git.CreateWorktree")() + // Create a temp dir for the worktree.
27-29: Consider wrapping the temp directory creation error with context.This would provide more actionable feedback if temp directory creation fails.
tempParentDir, err := os.MkdirTemp("", "atmos-worktree-") if err != nil { - return "", err + return "", errUtils.Build(errUtils.ErrCreateTempDir). + WithCause(err). + Err() }
59-66: Add performance tracking for public function.Per coding guidelines, public functions should include perf tracking.
func RemoveWorktree(repoDir, worktreePath string) { + defer perf.Track(nil, "git.RemoveWorktree")() + cmd := exec.Command("git", "worktree", "remove", "--force", worktreePath)
70-78: Usefilepath.Separatorfor cross-platform compatibility.The hardcoded
/inworktreeSuffixcould cause issues on Windows where the path separator is\.Consider using
filepath.Separatoror updating the logic:-const ( - worktreeSubdir = "worktree" - worktreeSuffix = "/" + worktreeSubdir -) +const worktreeSubdir = "worktree"func GetWorktreeParentDir(worktreePath string) string { - if strings.HasSuffix(worktreePath, worktreeSuffix) { - return strings.TrimSuffix(worktreePath, worktreeSuffix) + dir, base := filepath.Split(worktreePath) + if base == worktreeSubdir { + return filepath.Clean(dir) } return worktreePath }pkg/ui/spinner/spinner.go (1)
278-296: Consider Start completion timing.If
Start()returns before the goroutine fully initializess.program, andSuccess()/Error()is called immediately, there's a theoretical race. Currently,s.programassignment happens before the goroutine starts, so this should be safe, but documenting or clarifying the expected usage pattern (i.e., don't callSuccessimmediately afterStart) would help.cmd/list/affected.go (1)
116-146: Consider adding perf tracking.Per coding guidelines, functions should include
defer perf.Track(...)(). While this is a private function, the execution path would benefit from tracking for performance analysis.Also,
opts.Formatis populated from Viper (line 64) but not passed toAffectedCommandOptions. The format is re-read fromcmd.Flags()inExecuteListAffectedCmd(line 88 of list_affected.go). This works but is slightly redundant.pkg/list/list_affected.go (2)
268-277: Consider logging column parse errors.When
parseAffectedColumnsFlagfails (line 273-274), the error is silently swallowed and defaults are used. This fallback is sensible, but logging the parse error at debug/trace level would help users troubleshoot malformed--columnsvalues.// If --columns flag is provided, parse it (user takes responsibility for format). if len(columnsFlag) > 0 { columns, err := parseAffectedColumnsFlag(columnsFlag) - if err == nil && len(columns) > 0 { + if err != nil { + log.Debug("Failed to parse columns flag, using defaults", "error", err) + } else if len(columns) > 0 { return columns } }
304-345: Parsing could usestrings.SplitN.The manual loop to find
=(lines 316-322) could be simplified withstrings.SplitN(spec, "=", 2). Current implementation works, just a readability suggestion.+import "strings" + // parseAffectedColumnsFlag parses column specifications from CLI flag. // Each flag value should be in the format: "Name=TemplateExpression". func parseAffectedColumnsFlag(columnsFlag []string) ([]column.Config, error) { defer perf.Track(nil, "list.parseAffectedColumnsFlag")() if len(columnsFlag) == 0 { return tableAffectedColumns, nil } columns := make([]column.Config, 0, len(columnsFlag)) for _, spec := range columnsFlag { - // Split on first '=' to separate name from template. - idx := -1 - for i, c := range spec { - if c == '=' { - idx = i - break - } - } - if idx == -1 { + parts := strings.SplitN(spec, "=", 2) + if len(parts) != 2 { continue } - name := spec[:idx] - value := spec[idx+1:] + name := parts[0] + value := parts[1] if name == "" || value == "" { continue }
osterman
force-pushed
the
osterman/list-affected-command
branch
from
6d8f864 to
7cfcdf2
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
internal/exec/describe_affected_helpers.go (1)
231-270: Consider extracting cleanup helper to reduce repetition.The cleanup sequence
RemoveWorktree+removeTempDir(GetWorktreeParentDir(...))appears 4 times. A small helper would reduce duplication while respecting the Windows defer concerns:func cleanupWorktree(repoDir, worktreePath string) { g.RemoveWorktree(repoDir, worktreePath) removeTempDir(g.GetWorktreeParentDir(worktreePath)) }Not blocking since the current approach is correct.
pkg/list/list_affected.go (1)
302-343: Column parsing could usestrings.Cutfor cleaner splitting.Minor simplification using Go 1.18+
strings.Cut:- // Split on first '=' to separate name from template. - idx := -1 - for i, c := range spec { - if c == '=' { - idx = i - break - } - } - if idx == -1 { - continue - } - - name := spec[:idx] - value := spec[idx+1:] + name, value, found := strings.Cut(spec, "=") + if !found { + continue + }Not blocking.
internal/exec/terraform_output_utils.go (1)
624-627: Consider removing explicit Stop() calls.With
defer s.Stop()on line 581, these explicit calls are redundant. If you makeStop()idempotent (as suggested above), this code is safe but unnecessary. Otherwise, you could simplify by removing these explicit calls.if err != nil { - if s != nil { - s.Stop() - } return nil, false, fmt.Errorf("failed to describe the component %s in the stack %s: %w", component, stack, err) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (21)
cmd/list/affected.go(1 hunks)cmd/list/flag_wrappers.go(1 hunks)cmd/list/list.go(1 hunks)docs/fixes/list-affected-false-positives-cache-contamination.md(1 hunks)errors/errors.go(1 hunks)internal/exec/describe_affected_helpers.go(3 hunks)internal/exec/describe_affected_utils.go(1 hunks)internal/exec/spinner_utils.go(0 hunks)internal/exec/terraform_output_utils.go(5 hunks)internal/exec/validate_component.go(2 hunks)internal/exec/validate_stacks.go(3 hunks)pkg/git/worktree.go(1 hunks)pkg/list/extract/affected.go(1 hunks)pkg/list/extract/affected_test.go(1 hunks)pkg/list/extract/metadata.go(4 hunks)pkg/list/extract/metadata_test.go(3 hunks)pkg/list/list_affected.go(1 hunks)pkg/ui/formatter.go(1 hunks)pkg/ui/spinner/spinner.go(4 hunks)website/blog/2025-12-15-list-affected-command.mdx(1 hunks)website/docs/cli/commands/list/list-affected.mdx(1 hunks)
💤 Files with no reviewable changes (1)
- internal/exec/spinner_utils.go
🚧 Files skipped from review as they are similar to previous changes (5)
- cmd/list/list.go
- errors/errors.go
- pkg/list/extract/metadata.go
- internal/exec/validate_stacks.go
- internal/exec/validate_component.go
🧰 Additional context used
📓 Path-based instructions (7)
**/*.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: Comment style: All comments must end with periods (enforced bygodotlinter)
Import organization: Three groups separated by blank lines (stdlib, 3rd-party, Atmos packages), sorted alphabetically within groups, maintain aliases:cfg,log,u,errUtils
Performance tracking: Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions. Usenilif no atmosConfig param
Error handling: ALL user-facing errors MUST use ErrorBuilder with sentinel errors fromerrors/errors.go. ALWAYS useerrors.Is()for checking, NEVER string comparison. ALWAYS useassert.ErrorIs()in tests, NEVERassert.Contains(err.Error(), ...)
Mock generation: Usego.uber.org/mock/mockgenwith//go:generatedirectives. Never manual mocks
File organization: Keep files small and focused (<600 lines). One cmd/impl per file. Co-locate tests. Never use `//revive:disable:file-lengt...
Files:
pkg/list/extract/metadata_test.gointernal/exec/describe_affected_helpers.gopkg/ui/formatter.gocmd/list/flag_wrappers.gopkg/ui/spinner/spinner.gopkg/list/extract/affected.gocmd/list/affected.gopkg/list/list_affected.gointernal/exec/terraform_output_utils.gopkg/list/extract/affected_test.gopkg/git/worktree.gointernal/exec/describe_affected_utils.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: Test skipping conventions: Uset.Skipf("reason")with clear context. CLI tests auto-build temp binaries
Golden snapshots: NEVER manually edit golden snapshot files - Always use-regenerate-snapshotsflag. Snapshots capture exact output including invisible formatting (lipgloss padding, ANSI codes, trailing whitespace). Different environments produce different output. Never use pipe redirection (2>&1,| head,| tail) when running tests as piping breaks TTY detection
Files:
pkg/list/extract/metadata_test.gopkg/list/extract/affected_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Files:
cmd/list/flag_wrappers.gocmd/list/affected.go
cmd/*/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/*/*.go: Flag handling: Commands MUST useflags.NewStandardParser()for command-specific flags. NEVER callviper.BindEnv()orviper.BindPFlag()directly (enforced by Forbidigo linter)
CLI commands structure: Embed examples fromcmd/markdown/*_usage.mdusing//go:embed. Render withutils.PrintfMarkdown()
Files:
cmd/list/flag_wrappers.gocmd/list/affected.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in thewebsite/directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Keep website code in thewebsite/directory, follow the existing website architecture and style, and test website changes locally before committing
Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Files:
website/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
website/blog/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
Blog posts: Use
.mdxformat with YAML front matter inwebsite/blog/YYYY-MM-DD-feature-name.mdx. Include<!--truncate-->after intro. Use only tags defined inwebsite/blog/tags.yml. Use authors fromwebsite/blog/authors.yml
Files:
website/blog/2025-12-15-list-affected-command.mdx
website/docs/cli/commands/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation requirements: CLI command docs MUST include: 1) Frontmatter (title, sidebar_label, sidebar_class_name, id, description), 2) Intro component, 3) Screengrab, 4) Usage section with shell code block, 5) Arguments/Flags using
<dl><dt>and<dd>, 6) Examples section. File location:website/docs/cli/commands/<command>/<subcommand>.mdx
Files:
website/docs/cli/commands/list/list-affected.mdx
🧠 Learnings (69)
📓 Common learnings
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: 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: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Adding new CLI command: 1) Create `cmd/[command]/` with CommandProvider interface, 2) Add blank import to `cmd/root.go`: `_ "github.com/cloudposse/atmos/cmd/mycommand"`, 3) Implement in `internal/exec/mycommand.go`, 4) Add tests in `cmd/mycommand/mycommand_test.go`, 5) Create Docusaurus docs in `website/docs/cli/commands/<command>/<subcommand>.mdx`, 6) Build website: `cd website && npm run build`. See `docs/developing-atmos-commands.md` and `docs/prd/command-registry-pattern.md`
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: 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 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: 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: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: website/docs/cheatsheets/vendoring.mdx:70-70
Timestamp: 2024-11-12T13:06:56.194Z
Learning: In `atmos vendor pull --everything`, the `--everything` flag uses the TTY for TUI but is not interactive.
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to README.md : Update README.md with new commands and features
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).
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Test quality: Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests - don't test hardcoded stubs return hardcoded values. Make code testable - use DI to avoid `os.Exit`, `CheckErrorPrintAndExit`, external systems. No coverage theater - validate real behavior. Remove always-skipped tests - fix or delete. Table-driven tests need real scenarios. Use `assert.ErrorIs(err, ErrSentinel)` for our/stdlib errors. String matching OK for third-party errors
Applied to files:
pkg/list/extract/metadata_test.gopkg/list/extract/affected_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/list/extract/metadata_test.gopkg/list/extract/affected_test.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
internal/exec/describe_affected_helpers.gocmd/list/flag_wrappers.godocs/fixes/list-affected-false-positives-cache-contamination.mdpkg/list/extract/affected.gocmd/list/affected.gopkg/list/list_affected.gopkg/list/extract/affected_test.gointernal/exec/describe_affected_utils.gowebsite/docs/cli/commands/list/list-affected.mdx
📚 Learning: 2025-12-13T06:10:19.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: internal/exec/workflow_utils.go:0-0
Timestamp: 2025-12-13T06:10:19.505Z
Learning: Atmos workflows: In internal/exec/workflow_utils.go ExecuteWorkflow, non-identity steps intentionally use baseWorkflowEnv, which is constructed from the parent environment with PATH modifications for the toolchain. Avoid appending os.Environ() again; prefer documenting this behavior and testing that standard environment variables are preserved.
Applied to files:
internal/exec/describe_affected_helpers.gopkg/git/worktree.gointernal/exec/describe_affected_utils.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : I/O and UI usage: Separate I/O (streams) from UI (formatting). Use `data.*` for pipeable output to stdout, `ui.*` for human messages to stderr. Never use `fmt.Fprintf(os.Stdout/Stderr, ...)` or direct stream access. For data: `data.Write/Writef/Writeln/WriteJSON/WriteYAML`. For UI: `ui.Write*/Success/Error/Warning/Info`. For markdown: `ui.Markdown/MarkdownMessage`
Applied to files:
pkg/ui/formatter.gopkg/ui/spinner/spinner.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:
pkg/ui/formatter.gopkg/ui/spinner/spinner.gointernal/exec/terraform_output_utils.gointernal/exec/describe_affected_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 : 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/list/flag_wrappers.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to cmd/*/*.go : Flag handling: Commands MUST use `flags.NewStandardParser()` for command-specific flags. NEVER call `viper.BindEnv()` or `viper.BindPFlag()` directly (enforced by Forbidigo linter)
Applied to files:
cmd/list/flag_wrappers.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/list/flag_wrappers.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/list/flag_wrappers.gocmd/list/affected.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:
docs/fixes/list-affected-false-positives-cache-contamination.mdinternal/exec/describe_affected_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: 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:
docs/fixes/list-affected-false-positives-cache-contamination.mdinternal/exec/describe_affected_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: 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:
docs/fixes/list-affected-false-positives-cache-contamination.mdinternal/exec/describe_affected_utils.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:
docs/fixes/list-affected-false-positives-cache-contamination.md
📚 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:
docs/fixes/list-affected-false-positives-cache-contamination.mdpkg/list/list_affected.gointernal/exec/describe_affected_utils.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:
docs/fixes/list-affected-false-positives-cache-contamination.mdinternal/exec/terraform_output_utils.gointernal/exec/describe_affected_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: 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/fixes/list-affected-false-positives-cache-contamination.mdinternal/exec/describe_affected_utils.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:
docs/fixes/list-affected-false-positives-cache-contamination.md
📚 Learning: 2025-12-13T06:10:06.715Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:06.715Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Applied to files:
docs/fixes/list-affected-false-positives-cache-contamination.md
📚 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/fixes/list-affected-false-positives-cache-contamination.md
📚 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:
docs/fixes/list-affected-false-positives-cache-contamination.md
📚 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:
docs/fixes/list-affected-false-positives-cache-contamination.md
📚 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 meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Applied to files:
pkg/ui/spinner/spinner.gointernal/exec/terraform_output_utils.go
📚 Learning: 2024-11-10T18:37:10.032Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_component_utils.go:354-360
Timestamp: 2024-11-10T18:37:10.032Z
Learning: In the vendoring process, a TTY can exist without being interactive. If the process does not prompt the user, we should not require interactive mode to display the TUI. The `CheckTTYSupport` function should check TTY support on stdout rather than stdin.
Applied to files:
pkg/ui/spinner/spinner.go
📚 Learning: 2024-11-10T19:28:17.365Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:0-0
Timestamp: 2024-11-10T19:28:17.365Z
Learning: When TTY is not supported, log the downgrade message at the Warn level using `u.LogWarning(cliConfig, ...)` instead of `fmt.Println`.
Applied to files:
pkg/ui/spinner/spinner.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:
pkg/ui/spinner/spinner.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/list/affected.gowebsite/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
cmd/list/affected.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/list/affected.gopkg/list/list_affected.go
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/list/affected.gointernal/exec/describe_affected_utils.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/list/affected.gointernal/exec/describe_affected_utils.go
📚 Learning: 2025-09-25T01:02:48.697Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/manager.go:304-312
Timestamp: 2025-09-25T01:02:48.697Z
Learning: The auth manager in pkg/auth/manager.go should remain cloud-agnostic and not contain AWS-specific logic or references to specific cloud providers. Keep the manager generic and extensible.
Applied to files:
pkg/list/list_affected.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:
pkg/list/list_affected.go
📚 Learning: 2025-09-09T02:14:36.708Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: internal/auth/types/whoami.go:14-15
Timestamp: 2025-09-09T02:14:36.708Z
Learning: The WhoamiInfo struct in internal/auth/types/whoami.go requires the Credentials field to be JSON-serializable for keystore unmarshaling operations, despite security concerns about credential exposure.
Applied to files:
pkg/list/list_affected.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:
pkg/list/list_affected.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
pkg/list/list_affected.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : Import organization: Three groups separated by blank lines (stdlib, 3rd-party, Atmos packages), sorted alphabetically within groups, maintain aliases: `cfg`, `log`, `u`, `errUtils`
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : Performance tracking: Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions. Use `nil` if no atmosConfig param
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : Theme: Use colors from `pkg/ui/theme/colors.go`
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
internal/exec/terraform_output_utils.gointernal/exec/describe_affected_utils.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : Environment variables: Use `viper.BindEnv("ATMOS_VAR", "ATMOS_VAR", "FALLBACK")` - ATMOS_ prefix required
Applied to files:
internal/exec/terraform_output_utils.go
📚 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:
internal/exec/terraform_output_utils.go
📚 Learning: 2024-12-03T05:18:49.169Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2024-12-03T05:29:07.718Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:145-146
Timestamp: 2024-12-03T05:29:07.718Z
Learning: In the Atmos project, a 5-minute timeout in the `execTerraformOutput` function is acceptable for retrieving `terraform output` for a component in a stack.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to internal/exec/template_funcs.go : Template functions: Implement in `internal/exec/template_funcs.go`, register, test, document. Available: `atmos.Component/Stack/Setting()`, `terraform.output/state()`, `store.get()`, `exec()`, `env()`
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2024-11-24T19:13:10.287Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:407-416
Timestamp: 2024-11-24T19:13:10.287Z
Learning: In `internal/exec/terraform_clean.go`, when `getStackTerraformStateFolder` returns an error in the `handleCleanSubCommand` function, the error is logged, and the process continues without returning the error.
Applied to files:
internal/exec/terraform_output_utils.go
📚 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:
internal/exec/terraform_output_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 **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/list/extract/affected_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 {go.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Applied to files:
pkg/git/worktree.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/describe_affected_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:
internal/exec/describe_affected_utils.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:
internal/exec/describe_affected_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:
internal/exec/describe_affected_utils.go
📚 Learning: 2025-12-13T04:37:18.265Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/root.go:0-0
Timestamp: 2025-12-13T04:37:18.265Z
Learning: In Atmos cmd/root.go Execute(), after cfg.InitCliConfig, we must call both toolchainCmd.SetAtmosConfig(&atmosConfig) and toolchain.SetAtmosConfig(&atmosConfig) so the CLI wrapper and the toolchain package receive configuration; missing either can cause nil-pointer panics in toolchain path resolution.
Applied to files:
internal/exec/describe_affected_utils.go
📚 Learning: 2025-09-13T16:39:20.007Z
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.
Applied to files:
website/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to website/docs/cli/commands/**/*.mdx : Documentation requirements: CLI command docs MUST include: 1) Frontmatter (title, sidebar_label, sidebar_class_name, id, description), 2) Intro component, 3) Screengrab, 4) Usage section with shell code block, 5) Arguments/Flags using `<dl><dt>` and `<dd>`, 6) Examples section. File location: `website/docs/cli/commands/<command>/<subcommand>.mdx`
Applied to files:
website/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Adding new CLI command: 1) Create `cmd/[command]/` with CommandProvider interface, 2) Add blank import to `cmd/root.go`: `_ "github.com/cloudposse/atmos/cmd/mycommand"`, 3) Implement in `internal/exec/mycommand.go`, 4) Add tests in `cmd/mycommand/mycommand_test.go`, 5) Create Docusaurus docs in `website/docs/cli/commands/<command>/<subcommand>.mdx`, 6) Build website: `cd website && npm run build`. See `docs/developing-atmos-commands.md` and `docs/prd/command-registry-pattern.md`
Applied to files:
website/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
📚 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 README.md : Update README.md with new commands and features
Applied to files:
website/blog/2025-12-15-list-affected-command.mdx
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Documentation (MANDATORY): All cmds/flags need Docusaurus docs in `website/docs/cli/commands/`. Use `<dl>` for args/flags. Build: `cd website && npm run build`. Verify links by finding doc file, checking slug in frontmatter, verifying existing links. Never use command name vs. filename or guess URLs
Applied to files:
website/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to website/blog/*.mdx : Blog posts: Use `.mdx` format with YAML front matter in `website/blog/YYYY-MM-DD-feature-name.mdx`. Include `<!--truncate-->` after intro. Use only tags defined in `website/blog/tags.yml`. Use authors from `website/blog/authors.yml`
Applied to files:
website/blog/2025-12-15-list-affected-command.mdx
📚 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 website/** : Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Applied to files:
website/blog/2025-12-15-list-affected-command.mdx
📚 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/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
📚 Learning: 2025-12-13T06:07:34.794Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Applied to files:
website/blog/2025-12-15-list-affected-command.mdx
📚 Learning: 2025-09-27T20:50:20.564Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
Applied to files:
website/blog/2025-12-15-list-affected-command.mdx
🧬 Code graph analysis (8)
internal/exec/describe_affected_helpers.go (3)
pkg/logger/log.go (1)
Debug(24-26)pkg/git/worktree.go (3)
CreateWorktree(22-54)RemoveWorktree(59-66)GetWorktreeParentDir(70-78)pkg/git/git.go (2)
OpenWorktreeAwareRepo(176-185)GetRepoConfig(28-48)
pkg/ui/spinner/spinner.go (4)
pkg/ui/formatter.go (4)
Success(193-200)FormatError(324-331)FormatSuccess(313-320)Error(215-222)pkg/terminal/terminal.go (1)
EscResetLine(24-24)internal/tui/templates/term/term_writer.go (1)
IsTTYSupportForStdout(121-123)pkg/io/global.go (1)
UI(33-33)
cmd/list/affected.go (3)
cmd/list/flag_wrappers.go (4)
NewListParser(414-425)WithFormatFlag(32-40)WithStackFlag(126-133)WithSkipFlag(395-402)internal/exec/cli_utils.go (1)
ProcessCommandLineArgs(80-201)pkg/list/list_affected.go (2)
ExecuteListAffectedCmd(74-124)AffectedCommandOptions(45-71)
pkg/list/list_affected.go (7)
pkg/schema/schema.go (2)
ConfigAndStacksInfo(646-742)AtmosConfiguration(53-96)pkg/config/config.go (1)
InitCliConfig(28-67)pkg/ui/spinner/spinner.go (2)
ExecWithSpinnerDynamic(131-176)New(270-275)pkg/list/extract/affected.go (1)
Affected(12-29)pkg/list/column/column.go (2)
NewSelector(52-79)BuildColumnFuncMap(278-304)internal/exec/describe_affected_helpers.go (3)
ExecuteDescribeAffectedWithTargetRepoPath(277-337)ExecuteDescribeAffectedWithTargetRefClone(28-181)ExecuteDescribeAffectedWithTargetRefCheckout(187-273)pkg/list/sort/sort.go (5)
String(27-27)Sorter(35-39)ParseSortSpec(195-237)NewSorter(48-54)Ascending(17-17)
internal/exec/terraform_output_utils.go (1)
pkg/ui/spinner/spinner.go (2)
Spinner(262-267)New(270-275)
pkg/list/extract/affected_test.go (2)
pkg/list/extract/affected.go (1)
Affected(12-29)pkg/schema/schema.go (1)
Dependent(877-892)
pkg/git/worktree.go (1)
errors/errors.go (1)
ErrGitRefNotFound(157-157)
internal/exec/describe_affected_utils.go (1)
internal/exec/stack_processor_cache.go (1)
ClearBaseComponentConfigCache(158-164)
🪛 LanguageTool
docs/fixes/list-affected-false-positives-cache-contamination.md
[style] ~21-~21: Consider using the typographical ellipsis character here instead.
Context: ...erraform stack.metadata ``` Even when git diff refs/remotes/origin/HEAD...refs/heads/feature-branch shows no diff...
(ELLIPSIS)
[typographical] ~45-~45: Consider using typographic quotation marks here.
Context: ..., causing all local stacks to appear as "affected" since they had no remote counterpart to...
(EN_QUOTES)
[typographical] ~116-~116: To join two clauses or introduce examples, consider using an em dash.
Context: ...nternal/exec/describe_affected_utils.go- Main fix location -internal/exec/stack...
(DASH_RULE)
website/blog/2025-12-15-list-affected-command.mdx
[style] ~35-~35: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...nge, understanding what components were affected by your changes required parsing JSON o...
(EN_REPEATEDWORDS_AFFECT)
[typographical] ~43-~43: To join two clauses or introduce examples, consider using an em dash.
Context: ...different workflows: - table (default) - Human-readable overview - json / `yaml...
(DASH_RULE)
[typographical] ~44-~44: To join two clauses or introduce examples, consider using an em dash.
Context: ...uman-readable overview - json / yaml - Machine-readable for automation - csv ...
(DASH_RULE)
[typographical] ~45-~45: To join two clauses or introduce examples, consider using an em dash.
Context: ...-readable for automation - csv / tsv - Spreadsheet and scripting integration - ...
(DASH_RULE)
website/docs/cli/commands/list/list-affected.mdx
[typographical] ~172-~172: To join two clauses or introduce examples, consider using an em dash.
Context: ...lable for custom columns: - component - Component name - component_type - Comp...
(DASH_RULE)
[typographical] ~177-~177: To join two clauses or introduce examples, consider using an em dash.
Context: ...space, tenant, environment, stage- Stack naming parts -affected` - Primar...
(DASH_RULE)
[typographical] ~222-~222: To join two clauses or introduce examples, consider using an em dash.
Context: ...ected`](/cli/commands/describe/affected) - Get detailed affected components in JSON...
(DASH_RULE)
⏰ 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). (4)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
osterman
force-pushed
the
osterman/list-affected-command
branch
from
7cfcdf2 to
71f3ead
Compare
osterman
force-pushed
the
osterman/list-affected-command
branch
from
71f3ead to
650e4e8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1874 +/- ##
==========================================
- Coverage 73.22% 73.18% -0.04%
==========================================
Files 609 612 +3
Lines 57009 57532 +523
==========================================
+ Hits 41742 42107 +365
- Misses 12328 12485 +157
- Partials 2939 2940 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
internal/exec/terraform_output_utils.go (1)
574-582: Verify spinner Stop() idempotency.The past review comment flagged that explicit
s.Stop()calls (lines 625, 647, 659) combined withdefer s.Stop()may cause issues ifStop()isn't idempotent. The concern was that sending to an already-terminated program could be problematic.Consider either:
- Removing the explicit
s.Stop()calls in error paths and relying on defer- Making
Stop()idempotent by settings.program = nilafter the first stop#!/bin/bash # Check if spinner.Stop() is idempotent ast-grep --pattern $'func (s *Spinner) Stop() { $$$ }'pkg/ui/spinner/spinner.go (1)
298-334: Stop(), Success(), and Error() are not idempotent.As flagged in previous reviews, these methods don't set
s.program = nilafter termination. CallingStop()twice orStop()afterSuccess()will block on an already-closed channel or send to a terminated program.Apply this diff to make them idempotent:
func (s *Spinner) Stop() { if s.program == nil { return } s.program.Send(manualStopMsg{}) <-s.done + s.program = nil } func (s *Spinner) Success(message string) { if !s.isTTY { _ = ui.Success(message) return } if s.program == nil { _ = ui.Success(message) return } s.program.Send(manualStopMsg{message: message, success: true}) <-s.done + s.program = nil } func (s *Spinner) Error(message string) { if !s.isTTY { _ = ui.Error(message) return } if s.program == nil { _ = ui.Error(message) return } s.program.Send(manualStopMsg{message: message, success: false}) <-s.done + s.program = nil }pkg/git/worktree.go (2)
12-15: Hardcoded path separator breaks Windows compatibility.The forward slash in
worktreeSuffixwon't work correctly on Windows. Per coding guidelines, usefilepath.Join()orfilepath.Separator.Apply this diff:
+import "path/filepath" + const ( worktreeSubdir = "worktree" - worktreeSuffix = "/" + worktreeSubdir + worktreeSuffix = string(filepath.Separator) + worktreeSubdir )As per coding guidelines: "Use
filepath.Join(), not hardcoded separators."
22-54: Add performance tracking and fix hardcoded path separator.Two issues:
- Missing performance tracking for this public function (per coding guidelines)
- Line 30 uses hardcoded
/which breaks on Windows (past comment)Apply this diff:
func CreateWorktree(repoDir, targetCommit string) (string, error) { + defer perf.Track(nil, "git.CreateWorktree")() + // Create a temp dir for the worktree. // Note: We create a parent temp dir and use a subdirectory because // git worktree add requires the target directory to not exist. tempParentDir, err := os.MkdirTemp("", "atmos-worktree-") if err != nil { return "", err } - worktreePath := tempParentDir + "/worktree" + worktreePath := filepath.Join(tempParentDir, worktreeSubdir)As per coding guidelines.
🧹 Nitpick comments (2)
pkg/list/extract/affected.go (1)
70-88: Consider pre-allocating result slice capacity.For deeply nested dependents, the slice grows incrementally. Minor optimization opportunity.
func flattenDependentsRecursive(dependents []schema.Dependent, depth int) []map[string]any { defer perf.Track(nil, "extract.flattenDependentsRecursive")() - result := make([]map[string]any, 0) + result := make([]map[string]any, 0, len(dependents)) for i := range dependents {internal/exec/describe_affected_helpers.go (1)
261-270: Consider extracting cleanup helper.The cleanup pattern
g.RemoveWorktree(...) + removeTempDir(g.GetWorktreeParentDir(...))appears 4 times in this function. A small helper could reduce duplication.// cleanupWorktree removes the worktree and its parent temp directory. func cleanupWorktree(repoPath, worktreePath string) { g.RemoveWorktree(repoPath, worktreePath) removeTempDir(g.GetWorktreeParentDir(worktreePath)) }
osterman
force-pushed
the
osterman/list-affected-command
branch
from
650e4e8 to
0b32948
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
internal/exec/terraform_output_utils.go (1)
574-665: Consider simplifying spinner lifecycle management.The spinner is initialized with
defer s.Stop()on line 581, but then explicitly stopped on error paths (lines 625-626, 647-648, 659-660). This could lead to double-stop issues ifStop()is not idempotent.Two options:
- Remove explicit
s.Stop()calls and rely solely ondefer- Make
Stop()idempotent (check if it's already stopped before sending messages)The current implementation works because Go's channel operations are safe, but it's not the clearest pattern.
Consider this simplification - remove explicit stops and rely on defer:
if err != nil { - if s != nil { - s.Stop() - } return nil, false, fmt.Errorf("failed to describe the component %s in the stack %s: %w", component, stack, err) }Apply similar changes to lines 647-648 and 659-660.
docs/fixes/list-affected-false-positives-cache-contamination.md (1)
1-126: Clear documentation of the bug fix.The documentation effectively explains the false positive issue, root cause analysis, and the fix approach. The technical details about path calculation from git root and cache hygiene are well documented.
Optional: The static analysis tool flagged minor typography items (typographic ellipsis on line 21, quotation marks on line 45, em dash on line 116), but these are cosmetic and can be handled in a separate documentation cleanup if desired.
pkg/list/list_affected.go (2)
302-343: Column parsing works but could be simplified.The
parseAffectedColumnsFlagfunction correctly parses theName=Templateformat. Lines 314-320 use a manual loop to find the first=, which works but could be simplified.Optional: Consider using
strings.Cut(Go 1.18+) for cleaner parsing:for _, spec := range columnsFlag { - // Split on first '=' to separate name from template. - idx := -1 - for i, c := range spec { - if c == '=' { - idx = i - break - } - } - if idx == -1 { + name, value, found := strings.Cut(spec, "=") + if !found { continue } - - name := spec[:idx] - value := spec[idx+1:] if name == "" || value == "" {
345-372: Filter parsing could use standard library helper.Similar to the column parsing,
buildAffectedFiltersuses manual loop iteration (lines 358-364) to find the separator.Optional: Consider using
strings.Cutfor consistency:- idx := -1 - for i, c := range filterSpec { - if c == ':' || c == '=' { - idx = i - break - } - } - if idx > 0 && idx < len(filterSpec)-1 { - field := filterSpec[:idx] - value := filterSpec[idx+1:] + // Try ':' first, then '=' for backwards compatibility. + field, value, found := strings.Cut(filterSpec, ":") + if !found { + field, value, found = strings.Cut(filterSpec, "=") + } + if found && field != "" && value != "" { filters = append(filters, filter.NewColumnFilter(field, value)) }
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
pkg/ui/spinner/spinner_test.go (2)
295-334: Same non-deterministic test pattern.These tests repeat the "accept either outcome" approach from
TestExecWithSpinner_NonTTY. Same issue: no actual behavior validation.
486-529: More non-deterministic tests.Lines 486-529 continue the pattern of tests that accept any outcome. These verify the code doesn't panic but don't validate correctness.
🧹 Nitpick comments (11)
pkg/ui/spinner/spinner_test.go (2)
130-167: Non-TTY tests don't validate behavior.These tests execute operations but explicitly ignore all error outcomes with
_ = err // Accept either outcome. This provides coverage without validating correctness. Per coding guidelines: "No coverage theater - validate real behavior."Consider one of:
- Mock the UI formatter to make outcomes deterministic and assert proper behavior
- Skip these tests with
t.Skipf("requires TTY")if TTY is essential- Remove if they don't validate meaningful behavior
Based on coding guidelines: test behavior, not coverage metrics.
532-584: Consider removing tautological tests.Lines 532-584 test trivial assignments: a constant equals
"\n"and message structs store their field values. Per coding guidelines, "avoid tautological tests."These aren't harmful but add maintenance overhead without validating meaningful behavior. Consider removing or consolidating.
cmd/list/affected_test.go (3)
31-89: Consider refactoring tautological struct tests.These tests set struct fields and verify they're set, which doesn't validate real behavior. Consider either testing how these options are used in actual operations or removing them if they don't add value beyond documenting the structure.
92-208: Table-driven structure is good, but tests are tautological.While the table-driven format is appropriate, these tests only verify field assignments without testing actual behavior. Consider testing how these options influence real operations instead.
324-436: More tautological struct tests.Similar to earlier comments, these tests only verify field assignments. Consider consolidating or refactoring to test actual behavior with these options.
pkg/list/list_affected_test.go (1)
411-442: Consider removing tautological struct tests.These tests only verify field assignments without testing how the structs are used in actual operations. They add minimal value and could be removed or replaced with behavior tests.
Also applies to: 472-510
internal/exec/describe_affected_utils_test.go (5)
460-546: Test doesn't validate meaningful behavior.Both test cases only check that no error occurred without validating that
includeSettingsactually changes the behavior. Consider either:
- Asserting that the
affectedresults differ based on the flag (if settings affect output), or- Removing the test if the flag's behavior is sufficiently covered elsewhere.
As per coding guidelines: avoid "coverage theater" tests that don't validate real behavior.
548-565: Minor: test name doesn't match implementation.The test name says "nil stacks" but actually uses
emptyStacks := map[string]any{}(empty maps, not nil). Consider renaming toTestFindAffectedWithEmptyStacksfor accuracy.
567-606: Test doesn't validate meaningful behavior.The test only verifies that processing doesn't error when
includeSpaceliftAdminStacks=true, without validating what effect the flag has. Consider asserting the expected behavior (e.g., that spacelift-admin components are included/excluded appropriately).
680-689: Remove tautological constant tests.Testing that constants equal their hardcoded values provides no value. Constants can't change at runtime, so these assertions always pass and don't validate any behavior. As per coding guidelines: "Avoid tautological tests - don't test hardcoded stubs return hardcoded values."
-// Test constants. -func TestDescribeAffectedConstants(t *testing.T) { - t.Run("shaString constant", func(t *testing.T) { - assert.Equal(t, "SHA", shaString) - }) - - t.Run("refString constant", func(t *testing.T) { - assert.Equal(t, "ref", refString) - }) -} -
693-695: Remove tautological error message test.The first test case just verifies that the error message equals its hardcoded definition. This is tautological and provides no value. The second test case (validating
errors.Iscompatibility) is valuable and should be kept.func TestRemoteRepoIsNotGitRepoError(t *testing.T) { - t.Run("error message is correct", func(t *testing.T) { - expectedMsg := "the target remote repo is not a Git repository. Check that it was initialized and has '.git' folder" - assert.Equal(t, expectedMsg, RemoteRepoIsNotGitRepoError.Error()) - }) - t.Run("error can be used with errors.Is", func(t *testing.T) { err := RemoteRepoIsNotGitRepoError assert.True(t, errors.Is(err, RemoteRepoIsNotGitRepoError))
📜 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 (5)
cmd/list/affected_test.go(1 hunks)internal/exec/describe_affected_utils_test.go(2 hunks)pkg/git/worktree_test.go(1 hunks)pkg/list/list_affected_test.go(1 hunks)pkg/ui/spinner/spinner_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pkg/git/worktree_test.go
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: Comment style: All comments must end with periods (enforced bygodotlinter)
Import organization: Three groups separated by blank lines (stdlib, 3rd-party, Atmos packages), sorted alphabetically within groups, maintain aliases:cfg,log,u,errUtils
Performance tracking: Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions. Usenilif no atmosConfig param
Error handling: ALL user-facing errors MUST use ErrorBuilder with sentinel errors fromerrors/errors.go. ALWAYS useerrors.Is()for checking, NEVER string comparison. ALWAYS useassert.ErrorIs()in tests, NEVERassert.Contains(err.Error(), ...)
Mock generation: Usego.uber.org/mock/mockgenwith//go:generatedirectives. Never manual mocks
File organization: Keep files small and focused (<600 lines). One cmd/impl per file. Co-locate tests. Never use `//revive:disable:file-lengt...
Files:
pkg/list/list_affected_test.gointernal/exec/describe_affected_utils_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Test skipping conventions: Uset.Skipf("reason")with clear context. CLI tests auto-build temp binaries
Golden snapshots: NEVER manually edit golden snapshot files - Always use-regenerate-snapshotsflag. Snapshots capture exact output including invisible formatting (lipgloss padding, ANSI codes, trailing whitespace). Different environments produce different output. Never use pipe redirection (2>&1,| head,| tail) when running tests as piping breaks TTY detection
Files:
pkg/list/list_affected_test.gointernal/exec/describe_affected_utils_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Files:
cmd/list/affected_test.go
cmd/*/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/*/*.go: Flag handling: Commands MUST useflags.NewStandardParser()for command-specific flags. NEVER callviper.BindEnv()orviper.BindPFlag()directly (enforced by Forbidigo linter)
CLI commands structure: Embed examples fromcmd/markdown/*_usage.mdusing//go:embed. Render withutils.PrintfMarkdown()
Files:
cmd/list/affected_test.go
cmd/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/**/*_test.go: Testing: Usecmd.NewTestKit(t)for cmd tests to auto-clean RootCmd state (flags, args)
Test isolation: ALWAYS usecmd.NewTestKit(t)for cmd tests to auto-clean RootCmd state (flags, args). Required for any test touching RootCmd
Files:
cmd/list/affected_test.go
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Adding new CLI command: 1) Create `cmd/[command]/` with CommandProvider interface, 2) Add blank import to `cmd/root.go`: `_ "github.com/cloudposse/atmos/cmd/mycommand"`, 3) Implement in `internal/exec/mycommand.go`, 4) Add tests in `cmd/mycommand/mycommand_test.go`, 5) Create Docusaurus docs in `website/docs/cli/commands/<command>/<subcommand>.mdx`, 6) Build website: `cd website && npm run build`. See `docs/developing-atmos-commands.md` and `docs/prd/command-registry-pattern.md`
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to README.md : Update README.md with new commands and features
📚 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:
pkg/list/list_affected_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_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:
pkg/list/list_affected_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_test.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
pkg/list/list_affected_test.gointernal/exec/describe_affected_utils_test.gocmd/list/affected_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/list/list_affected_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Test quality: Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests - don't test hardcoded stubs return hardcoded values. Make code testable - use DI to avoid `os.Exit`, `CheckErrorPrintAndExit`, external systems. No coverage theater - validate real behavior. Remove always-skipped tests - fix or delete. Table-driven tests need real scenarios. Use `assert.ErrorIs(err, ErrSentinel)` for our/stdlib errors. String matching OK for third-party errors
Applied to files:
pkg/list/list_affected_test.gointernal/exec/describe_affected_utils_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_test.go
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
pkg/list/list_affected_test.gocmd/list/affected_test.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/describe_affected_utils_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:
internal/exec/describe_affected_utils_test.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:
internal/exec/describe_affected_utils_test.go
📚 Learning: 2025-12-13T06:10:19.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: internal/exec/workflow_utils.go:0-0
Timestamp: 2025-12-13T06:10:19.505Z
Learning: Atmos workflows: In internal/exec/workflow_utils.go ExecuteWorkflow, non-identity steps intentionally use baseWorkflowEnv, which is constructed from the parent environment with PATH modifications for the toolchain. Avoid appending os.Environ() again; prefer documenting this behavior and testing that standard environment variables are preserved.
Applied to files:
internal/exec/describe_affected_utils_test.go
📚 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:
internal/exec/describe_affected_utils_test.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/describe_affected_utils_test.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/describe_affected_utils_test.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:
internal/exec/describe_affected_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/list/affected_test.go
📚 Learning: 2025-12-10T18:32:43.260Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:43.260Z
Learning: In cmd subpackages, tests should avoid using NewTestKit(t) unless tests actually interact with RootCmd (e.g., execute commands via RootCmd or modify RootCmd state). For structural tests that only verify command structure/flags without touching RootCmd, TestKit cleanup is unnecessary.
Applied to files:
cmd/list/affected_test.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Applied to files:
cmd/list/affected_test.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/list/affected_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to cmd/**/*_test.go : Test isolation: ALWAYS use `cmd.NewTestKit(t)` for cmd tests to auto-clean RootCmd state (flags, args). Required for any test touching RootCmd
Applied to files:
cmd/list/affected_test.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
cmd/list/affected_test.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/list/affected_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Testing strategy: Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with `go.uber.org/mock/mockgen`. Use table-driven tests for comprehensive coverage. Integration tests in `tests/` only when necessary. Target >80% coverage
Applied to files:
pkg/ui/spinner/spinner_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Interface-driven design: Define interfaces for all major functionality. Use dependency injection for testability. Generate mocks with `go.uber.org/mock/mockgen`. Avoid integration tests by mocking external dependencies
Applied to files:
pkg/ui/spinner/spinner_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*_test.go : Test skipping conventions: Use `t.Skipf("reason")` with clear context. CLI tests auto-build temp binaries
Applied to files:
pkg/ui/spinner/spinner_test.go
🧬 Code graph analysis (4)
pkg/list/list_affected_test.go (4)
pkg/list/list_affected.go (1)
AffectedCommandOptions(45-71)pkg/list/sort/sort.go (2)
Order(13-13)Ascending(17-17)pkg/schema/schema.go (1)
AtmosConfiguration(53-96)cmd/cmd_utils.go (1)
Contains(1244-1251)
internal/exec/describe_affected_utils_test.go (2)
pkg/schema/schema.go (2)
AtmosConfiguration(53-96)Terraform(367-379)internal/exec/describe_affected_helpers.go (1)
RemoteRepoIsNotGitRepoError(19-19)
cmd/list/affected_test.go (1)
cmd/list/affected.go (1)
AffectedOptions(16-41)
pkg/ui/spinner/spinner_test.go (3)
cmd/cmd_utils.go (1)
Contains(1244-1251)pkg/ui/spinner/spinner.go (1)
ExecWithSpinnerDynamic(134-181)pkg/ui/formatter.go (1)
Success(193-200)
⏰ 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 (macos)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (12)
cmd/list/affected_test.go (5)
12-18: Structural validation looks good.This test appropriately verifies the command metadata and initialization.
21-28: Good validation test.This tests actual argument validation behavior rather than just structure.
211-247: Good structural validation.These tests appropriately verify parser initialization and flag registration, which are important for command setup.
250-315: Excellent behavior test.This test validates actual flag/viper precedence logic with real scenarios. This is exactly the kind of behavior testing that adds value.
1-436: TestKit usage is correct.These tests appropriately don't use
cmd.NewTestKit(t)since they only verify command structure and don't interact with RootCmd. Based on learnings, TestKit is only required when tests actually execute commands via RootCmd or modify RootCmd state.pkg/list/list_affected_test.go (3)
16-149: Solid behavior tests with good coverage.These table-driven tests appropriately validate parsing logic and precedence rules with comprehensive edge cases.
151-342: Excellent behavior testing.These tests validate actual function behavior with comprehensive scenarios and edge cases. Good use of table-driven patterns.
344-470: Good tests for column handling.TestGetAffectedColumns validates format-based column selection behavior, and TestTableAndDataAffectedColumns provides appropriate structural validation of column definitions.
internal/exec/describe_affected_utils_test.go (4)
292-309: Test setup looks good.The updated configuration with realistic absolute paths properly exercises the filepath.Rel logic while still testing the error path. Good improvement over the previous minimal setup.
372-389: LGTM.Clean helper that enables testing HEAD error scenarios consistently.
608-642: LGTM.Good coverage of the error path when local repository HEAD lookup fails.
644-678: LGTM.Good symmetric coverage of the remote repository HEAD error path.
|
@aknysh I'll perform a full review of the PR. 🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
NOTICE (1)
1-3: Run the generate-notice.sh script to regenerate the NOTICE file.The pipeline indicates the file is out of date. Since the NOTICE file is programmatically generated from upstream dependencies, run
./scripts/generate-notice.shand commit the changes. License URL updates are handled automatically when package metadata is available.
🧹 Nitpick comments (1)
pkg/list/list_affected.go (1)
303-342: Column parsing is functional but could be simplified.The manual string parsing in
parseAffectedColumnsFlag(lines 313-322) works but could usestrings.SplitN(spec, "=", 2)for cleaner code. Current approach is functional, so this is optional.💡 Optional simplification
func parseAffectedColumnsFlag(columnsFlag []string) ([]column.Config, error) { defer perf.Track(nil, "list.parseAffectedColumnsFlag")() if len(columnsFlag) == 0 { return tableAffectedColumns, nil } columns := make([]column.Config, 0, len(columnsFlag)) for _, spec := range columnsFlag { - // Split on first '=' to separate name from template. - idx := -1 - for i, c := range spec { - if c == '=' { - idx = i - break - } - } - if idx == -1 { + parts := strings.SplitN(spec, "=", 2) + if len(parts) != 2 { continue } - name := spec[:idx] - value := spec[idx+1:] + name := parts[0] + value := parts[1] if name == "" || value == "" { continue }
📜 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 ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
NOTICE(4 hunks)go.mod(3 hunks)pkg/list/extract/affected_test.go(1 hunks)pkg/list/list_affected.go(1 hunks)pkg/list/renderer/renderer.go(2 hunks)pkg/ui/formatter_test.go(1 hunks)tests/snapshots/TestCLICommands_atmos_list_instances_no_tty.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_list_instances_with_custom_columns.stdout.golden(1 hunks)website/blog/2025-12-15-list-affected-command.mdx(1 hunks)website/docs/cli/commands/list/list-affected.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/snapshots/TestCLICommands_atmos_list_instances_no_tty.stdout.golden
- tests/snapshots/TestCLICommands_atmos_list_instances_with_custom_columns.stdout.golden
🧰 Additional context used
📓 Path-based instructions (6)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in thewebsite/directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Keep website code in thewebsite/directory, follow the existing website architecture and style, and test website changes locally before committing
Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Files:
website/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
website/blog/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
website/blog/**/*.mdx: Follow PR template (what/why/references); PRs labeled minor/major MUST include blog post at website/blog/YYYY-MM-DD-feature-name.mdx with YAML front matter, after intro, and only tags from website/blog/tags.yml
Blog posts MUST use only tags defined in website/blog/tags.yml and authors defined in website/blog/authors.yml; valid tags are: feature, enhancement, bugfix, dx, breaking-change, security, documentation, deprecation, core; never invent new tags
Files:
website/blog/2025-12-15-list-affected-command.mdx
**/*.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: NEVER use fmt.Fprintf(os.Stdout/Stderr) or fmt.Println(); use data.* or ui.* functions instead
All comments must end with periods (enforced by godot linter)
Organize imports in 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 for performance tracking; use nil if no atmosConfig param
All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Keep files small...
Files:
pkg/list/list_affected.gopkg/ui/formatter_test.gopkg/list/extract/affected_test.gopkg/list/renderer/renderer.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: Prefer unit tests with mocks over integration tests; use interfaces + dependency injection for testability; generate mocks with go.uber.org/mock/mockgen; use table-driven tests; target >80% coverage
Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Files:
pkg/ui/formatter_test.gopkg/list/extract/affected_test.go
website/docs/cli/commands/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
All CLI commands/flags need Docusaurus documentation in website/docs/cli/commands/ with specific structure: frontmatter, Intro component, Screengrab component, Usage section, Arguments/Flags using
- /
- , and Examples section
Files:
website/docs/cli/commands/list/list-affected.mdx
{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 (31)
📓 Common learnings
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: 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: 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: 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`.
📚 Learning: 2025-09-13T16:39:20.007Z
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.
Applied to files:
website/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
📚 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 README.md : Update README.md with new commands and features
Applied to files:
website/blog/2025-12-15-list-affected-command.mdx
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to website/docs/cli/commands/**/*.mdx : All CLI commands/flags need Docusaurus documentation in website/docs/cli/commands/ with specific structure: frontmatter, Intro component, Screengrab component, Usage section, Arguments/Flags using <dl><dt>/<dd>, and Examples section
Applied to files:
website/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.mdx
📚 Learning: 2025-09-27T20:50:20.564Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
Applied to files:
website/blog/2025-12-15-list-affected-command.mdx
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to website/blog/**/*.mdx : Follow PR template (what/why/references); PRs labeled minor/major MUST include blog post at website/blog/YYYY-MM-DD-feature-name.mdx with YAML front matter, <!--truncate--> after intro, and only tags from website/blog/tags.yml
Applied to files:
website/blog/2025-12-15-list-affected-command.mdx
📚 Learning: 2025-12-13T06:07:37.766Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Applied to files:
website/blog/2025-12-15-list-affected-command.mdxpkg/list/list_affected.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/blog/2025-12-15-list-affected-command.mdxwebsite/docs/cli/commands/list/list-affected.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/blog/2025-12-15-list-affected-command.mdxpkg/list/list_affected.gowebsite/docs/cli/commands/list/list-affected.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:
pkg/list/list_affected.gopkg/list/extract/affected_test.gowebsite/docs/cli/commands/list/list-affected.mdx
📚 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:
pkg/list/list_affected.go
📚 Learning: 2025-09-25T01:02:48.697Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/manager.go:304-312
Timestamp: 2025-09-25T01:02:48.697Z
Learning: The auth manager in pkg/auth/manager.go should remain cloud-agnostic and not contain AWS-specific logic or references to specific cloud providers. Keep the manager generic and extensible.
Applied to files:
pkg/list/list_affected.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:
pkg/list/list_affected.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:
pkg/list/list_affected.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:
pkg/list/list_affected.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:
pkg/list/list_affected.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:
pkg/list/list_affected.go
📚 Learning: 2025-12-13T06:10:13.688Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:13.688Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Applied to files:
pkg/list/list_affected.go
📚 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:
pkg/list/list_affected.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:
pkg/list/list_affected.go
📚 Learning: 2025-12-17T20:55:47.884Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1874
File: internal/exec/describe_affected_utils_test.go:436-468
Timestamp: 2025-12-17T20:55:47.884Z
Learning: In the Atmos codebase, there are two different paths for the `locked` flag: (1) filtering logic in `internal/exec/component_utils.go` (`isComponentLocked()`) reads from `componentSection["metadata"]["locked"]` to determine which components to include/exclude, and (2) extraction/rendering logic in `pkg/list/extract/affected.go` reads from `settings.metadata.locked` to display the locked status in output. Tests for filtering behavior should use `metadata.locked`.
Applied to files:
pkg/list/list_affected.gopkg/list/extract/affected_test.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
pkg/list/list_affected.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Applied to files:
pkg/ui/formatter_test.gopkg/list/extract/affected_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:
pkg/ui/formatter_test.gopkg/list/extract/affected_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/ui/formatter_test.gopkg/list/extract/affected_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/ui/formatter_test.gopkg/list/extract/affected_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 {go.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Applied to files:
NOTICEgo.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-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
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
🧬 Code graph analysis (2)
pkg/ui/formatter_test.go (3)
pkg/ui/formatter.go (6)
FormatSuccess(313-320)FormatError(324-331)InitFormatter(47-69)NewFormatter(386-395)Format(375-375)Errorf(226-233)pkg/terminal/terminal.go (1)
ColorNone(92-92)cmd/cmd_utils.go (1)
Contains(1257-1264)
pkg/list/extract/affected_test.go (3)
pkg/list/extract/affected.go (1)
Affected(12-29)pkg/schema/schema.go (1)
Dependent(898-913)cmd/cmd_utils.go (1)
Contains(1257-1264)
🪛 GitHub Actions: Dependency Review
NOTICE
[error] 1-3: NOTICE file is out of date. Run './scripts/generate-notice.sh' locally and commit the changes. (Command: 'git diff --exit-code NOTICE')
🪛 LanguageTool
website/blog/2025-12-15-list-affected-command.mdx
[style] ~35-~35: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...nge, understanding what components were affected by your changes required parsing JSON o...
(EN_REPEATEDWORDS_AFFECT)
[typographical] ~43-~43: To join two clauses or introduce examples, consider using an em dash.
Context: ...different workflows: - table (default) - Human-readable overview - json / `yaml...
(DASH_RULE)
[typographical] ~44-~44: To join two clauses or introduce examples, consider using an em dash.
Context: ...uman-readable overview - json / yaml - Machine-readable for automation - csv ...
(DASH_RULE)
[typographical] ~45-~45: To join two clauses or introduce examples, consider using an em dash.
Context: ...-readable for automation - csv / tsv - Spreadsheet and scripting integration #...
(DASH_RULE)
website/docs/cli/commands/list/list-affected.mdx
[typographical] ~172-~172: To join two clauses or introduce examples, consider using an em dash.
Context: ...lable for custom columns: - component - Component name - component_type - Comp...
(DASH_RULE)
[typographical] ~177-~177: To join two clauses or introduce examples, consider using an em dash.
Context: ...space, tenant, environment, stage- Stack naming parts -affected` - Primar...
(DASH_RULE)
[typographical] ~222-~222: To join two clauses or introduce examples, consider using an em dash.
Context: ...ected`](/cli/commands/describe/affected) - Get detailed affected components in JSON...
(DASH_RULE)
🔇 Additional comments (15)
pkg/ui/formatter_test.go (1)
1441-1544: LGTM! Solid test coverage for the new format helpers.The test properly covers
FormatSuccessandFormatErrorwith both initialized and non-initialized formatter states. The table-driven approach is clean, state management with locks and defers is correct, and the assertions verify behavior (output contains expected icon and text) rather than implementation details.pkg/list/renderer/renderer.go (2)
147-147: LGTM! Trailing newline improves output formatting.Adding the newline ensures JSON output follows standard conventions and displays cleanly in terminals.
177-177: LGTM! Consistent newline handling.Matches the JSON formatting approach and ensures delimited output follows conventions.
go.mod (1)
83-83: Routine dependency updates look good.The OPA and Google API version bumps align with the NOTICE file updates. These minor/patch updates support the new list-affected feature.
Also applies to: 106-106, 406-406
website/docs/cli/commands/list/list-affected.mdx (1)
1-225: Excellent documentation structure and content.The documentation follows the required Docusaurus format with frontmatter, Intro, Screengrab, Usage, Examples, Flags, and related sections. The progressive examples (basic → filtering → CI/CD) and the comparison table with
describe affectedare particularly helpful for users.website/blog/2025-12-15-list-affected-command.mdx (1)
1-120: Well-crafted blog post announcement.The post follows the required format with proper frontmatter, truncate marker, and valid tags. The content effectively communicates the value proposition with visual indicators, multiple formats, and practical CI/CD examples.
pkg/list/extract/affected_test.go (1)
11-388: Comprehensive test coverage with good structure.The test suite covers all key scenarios: empty inputs, single/multiple affected components, dependent flattening, nested dependencies, status indicators, and field presence. The table-driven test for status text states (lines 245-301) is particularly clean. Tests focus on behavior over implementation, meeting the >80% coverage guideline.
pkg/list/list_affected.go (8)
24-42: Format-aware column configurations are well-designed.The separation between
tableAffectedColumns(using colored status dot) anddataAffectedColumns(using semantic status text) properly handles the difference between interactive terminal output and machine-readable formats.
44-71: Well-organized options struct.The command options are logically grouped (git comparison, content, processing) with clear field names. Good to see the unused AuthManager was removed based on previous feedback.
74-124: Main execution flow is clean and user-friendly.The spinner with dynamic completion message showing compared refs (lines 108-109) provides excellent UX feedback. Empty result handling with a success message is more user-friendly than silence. Error wrapping throughout follows Go best practices.
127-157: Clean rendering pipeline.The extract → configure → filter → sort → render pipeline is well-structured with consistent error handling. Good reuse of existing renderer infrastructure.
168-248: Execution paths properly reuse existing logic.The three execution paths (RepoPath, CloneTargetRef, default checkout) correctly delegate to existing
ExecuteDescribeAffected*functions. SettingincludeSettingsto true ensures status indicators can be extracted.
254-264: Reference naming handles edge cases well.The nil
localRepoHeadhandling is properly documented (lines 252-253) and allows graceful degradation to a generic message. The RepoPath vs. normal ref distinction is clear.
268-299: Column selection and ref logic are straightforward.The precedence order (user columns flag → format-based defaults) respects user choice. The
selectRemoteReffunction has clear precedence logic with a sensible default.
345-397: Filter and sorter builders are clean.The filter builder is intentionally basic with room for extension (line 356 comment). The sorter builder properly delegates to existing
ParseSortSpecand provides sensible defaults (Stack → Component).
|
@aknysh I'll perform a comprehensive review of all the changes in this PR. 🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cmd/list/affected.go (1)
116-146: Missing performance tracking.Per coding guidelines, public functions should have
defer perf.Track(...)()at the start. Consider adding it here.🔎 Proposed fix
+import "github.com/cloudposse/atmos/pkg/perf" + func executeListAffectedCmd(cmd *cobra.Command, args []string, opts *AffectedOptions) error { + defer perf.Track(nil, "list.executeListAffectedCmd")() + // Process and validate command line arguments. configAndStacksInfo, err := e.ProcessCommandLineArgs("list", cmd, args, nil)pkg/list/list_affected.go (1)
301-342: Silent skip of invalid column specs.Invalid column specifications (missing
=, empty name/value) are silently skipped. This is forgiving for users but could mask typos. Consider logging a debug/trace message when specs are skipped so users can troubleshoot.🔎 Proposed enhancement
if idx == -1 { + log.Trace("Skipping invalid column spec (no '=' found)", "spec", spec) continue } name := spec[:idx] value := spec[idx+1:] if name == "" || value == "" { + log.Trace("Skipping invalid column spec (empty name or value)", "spec", spec) continue }
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)
NOTICE (1)
1-1: Run the generate-notice.sh script to sync the NOTICE file.The pipeline checks that the NOTICE file matches the output of
./scripts/generate-notice.sh. Run this script locally and commit the changes—the file is programmatically generated from your Go dependencies, so manual edits won't satisfy the pipeline.
🧹 Nitpick comments (4)
internal/exec/terraform_output_utils.go (1)
624-660: Consider removing explicit Stop() calls on error paths.The code has explicit
s.Stop()calls on error paths (lines 625, 647, 659) in addition todefer s.Stop()at line 581. Since Stop() is idempotent (per previous review), the explicit calls add defensive cleanup but may be redundant. Consider either:
- Remove explicit stops and rely solely on defer (cleaner)
- Add a comment explaining why explicit stops are preferred (defensive cleanup before error return)
The current implementation works correctly but the pattern isn't immediately clear.
pkg/list/extract/affected.go (1)
70-88: Consider depth limit for deeply nested dependents.The recursion works correctly. For extreme edge cases with very deep nesting, consider adding a max depth guard to prevent potential stack overflow. This is a minor defensive coding suggestion.
🔎 Optional depth limit
+const maxDependentDepth = 100 + func flattenDependentsRecursive(dependents []schema.Dependent, depth int) []map[string]any { defer perf.Track(nil, "extract.flattenDependentsRecursive")() + if depth > maxDependentDepth { + return nil + } + result := make([]map[string]any, 0)internal/exec/describe_affected_helpers.go (1)
225-245: Consider extracting cleanup logic.The worktree cleanup pattern (
g.RemoveWorktree+removeTempDir) is repeated in multiple error paths (lines 234-235, 242-243, 262-263, 269-270). Consider using a deferred cleanup helper for consistency and reduced duplication.🔎 Optional cleanup helper pattern
+ // Cleanup helper to remove worktree and temp dir. + cleanup := func() { + g.RemoveWorktree(localRepoInfo.LocalWorktreePath, worktreePath) + removeTempDir(g.GetWorktreeParentDir(worktreePath)) + } + // Open the worktree as a git repository. remoteRepo, err := g.OpenWorktreeAwareRepo(worktreePath) if err != nil { - g.RemoveWorktree(localRepoInfo.LocalWorktreePath, worktreePath) - removeTempDir(g.GetWorktreeParentDir(worktreePath)) + cleanup() return nil, nil, nil, "", errors.Join(err, RemoteRepoIsNotGitRepoError) }pkg/list/list_affected.go (1)
301-342: Consider using strings.SplitN for cleaner parsing.The manual loop to find
=works, butstrings.SplitN(spec, "=", 2)would be more idiomatic. Optional cleanup.🔎 Using strings.SplitN
for _, spec := range columnsFlag { - // Split on first '=' to separate name from template. - idx := -1 - for i, c := range spec { - if c == '=' { - idx = i - break - } - } - if idx == -1 { + // Split on first '=' to separate name from template. + parts := strings.SplitN(spec, "=", 2) + if len(parts) != 2 { continue } - name := spec[:idx] - value := spec[idx+1:] + name := parts[0] + value := parts[1]
Add performance tracking to executeListAffectedCmd function per coding guidelines, addressing CodeRabbit review feedback. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cmd/list/affected.go (2)
44-85: Consider embedding usage examples.Per coding guidelines, CLI commands should include examples embedded from
cmd/markdown/*_usage.mdfiles using//go:embedand rendered withutils.PrintfMarkdown(). This would improve the help text for users.Example pattern
Create
cmd/markdown/list_affected_usage.mdwith examples, then:+//go:embed markdown/list_affected_usage.md +var listAffectedUsage string + var affectedCmd = &cobra.Command{ Use: "affected", Short: "List affected components and stacks", - Long: "Display a table of affected components and stacks between Git commits.", + Long: utils.PrintfMarkdown(listAffectedUsage),
49-49: Remove redundant FParseErrWhitelist.
UnknownFlags: falseis the zero value and can be omitted entirely.Simplification
- FParseErrWhitelist: struct{ UnknownFlags bool }{UnknownFlags: false}, Args: cobra.NoArgs,
📜 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 (1)
cmd/list/affected.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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: Commands MUST use flags.NewStandardParser() for command-specific flags; NEVER call viper.BindEnv() or viper.BindPFlag() directly (Forbidigo enforces this); see cmd/version/version.go for reference
Embed CLI examples from cmd/markdown/*_usage.md using //go:embed; render with utils.PrintfMarkdown()
Files:
cmd/list/affected.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: NEVER use fmt.Fprintf(os.Stdout/Stderr) or fmt.Println(); use data.* or ui.* functions instead
All comments must end with periods (enforced by godot linter)
Organize imports in 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 for performance tracking; use nil if no atmosConfig param
All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Keep files small...
Files:
cmd/list/affected.go
🧠 Learnings (17)
📓 Common learnings
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
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.
📚 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:
cmd/list/affected.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/list/affected.go
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
cmd/list/affected.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/list/affected.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/list/affected.go
📚 Learning: 2025-11-30T04:16:24.155Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:34-48
Timestamp: 2025-11-30T04:16:24.155Z
Learning: In the cloudposse/atmos repository, the `defer perf.Track()` guideline applies to functions that perform meaningful work (I/O, computation, external calls), but explicitly excludes trivial accessors/mutators (e.g., simple getters, setters with single integer increments, string joins, or map appends) where the tracking overhead would exceed the actual method cost and provide no actionable performance data. Hot-path methods called in tight loops should especially avoid perf.Track() if they perform only trivial operations.
Applied to files:
cmd/list/affected.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
cmd/list/affected.go
📚 Learning: 2025-11-30T04:16:01.899Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:50-59
Timestamp: 2025-11-30T04:16:01.899Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` should NOT be added to trivial O(1) getter methods that only return field references or check map lengths (e.g., `GetDeferredValues()`, `HasDeferredValues()`). The guideline to add perf tracking to "all public functions" applies to functions that do meaningful work (I/O, computation, external calls), not to trivial accessors where the tracking overhead would exceed the operation time and pollute performance reports.
Applied to files:
cmd/list/affected.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions for performance tracking; use nil if no atmosConfig param
Applied to files:
cmd/list/affected.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 meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Applied to files:
cmd/list/affected.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:
cmd/list/affected.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/list/affected.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/list/affected.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/list/affected.go
📚 Learning: 2025-12-13T03:21:35.786Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:35.786Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/list/affected.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/list/affected.go
🧬 Code graph analysis (1)
cmd/list/affected.go (3)
cmd/list/flag_wrappers.go (6)
NewListParser(414-425)WithFormatFlag(32-40)WithStackFlag(126-133)WithProcessTemplatesFlag(252-259)WithProcessFunctionsFlag(263-270)WithSkipFlag(395-402)internal/exec/cli_utils.go (1)
ProcessCommandLineArgs(80-201)pkg/list/list_affected.go (2)
ExecuteListAffectedCmd(74-124)AffectedCommandOptions(45-71)
⏰ 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). (4)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Summary
🔇 Additional comments (1)
cmd/list/affected.go (1)
1-149: Solid implementation, follows coding guidelines.The command structure is clean and follows the established patterns:
- Proper import organization
- Correct flag handling with StandardParser (no direct Viper binding)
- Performance tracking added
- Error handling via returns (no os.Exit)
- Delegates to
list.ExecuteListAffectedCmdwith properly mapped options
…into osterman/list-affected-command
aknysh
dismissed stale reviews from coderabbitai[bot] and themself
via
76ba74e
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
internal/exec/describe_affected_helpers.go (1)
267-271: Log cleanup failures for debugging.Since this runs in defer, errors from
RemoveWorktreeorremoveTempDirare silently discarded. Consider logging failures so operators can diagnose issues like permission errors or locked files.🔎 Proposed enhancement
func cleanupWorktree(repoPath, worktreePath string) { - g.RemoveWorktree(repoPath, worktreePath) - removeTempDir(g.GetWorktreeParentDir(worktreePath)) + if err := g.RemoveWorktree(repoPath, worktreePath); err != nil { + log.Debug("Failed to remove worktree", "path", worktreePath, "error", err) + } + parentDir := g.GetWorktreeParentDir(worktreePath) + if err := os.RemoveAll(parentDir); err != nil { + log.Debug("Failed to remove worktree parent directory", "path", parentDir, "error", err) + } }pkg/ui/spinner/spinner_test.go (5)
531-536: Consider removing this tautological test.Testing that a constant equals its literal value doesn't verify behavior. Per coding guidelines, avoid tests that don't add meaningful coverage.
🔎 Suggested removal
-// Test newline constant. -func TestNewlineConstant(t *testing.T) { - t.Run("newline has expected value", func(t *testing.T) { - assert.Equal(t, "\n", newline) - }) -}
538-584: Message struct tests provide minimal value.These tests just verify struct field assignment works, which Go guarantees. If keeping for coverage metrics, that's fine, but they don't test behavior.
586-595: Test doesn't verify what the name claims.
TestSpinner_IsTTYonly assertssis not nil. TheisTTYfield is never verified. Either rename or add a meaningful assertion.🔎 Suggested fix
func TestSpinner_IsTTY(t *testing.T) { t.Run("spinner stores isTTY state", func(t *testing.T) { s := New("Testing") - // In test environment, isTTY is typically false. - // We verify the field is properly set. - assert.NotNil(t, s) - // isTTY is determined by term.IsTTYSupportForStdout(). + // In test environment, isTTY is typically false. + assert.False(t, s.isTTY, "expected non-TTY in test environment") }) }
673-739: Duplicate test coverage across field tests.
TestSpinner_Fields,TestSpinnerModel_Fields,TestDynamicSpinnerModel_Fields, andTestManualSpinnerModel_Fieldslargely duplicate the correspondingTestNew*tests earlier in the file. Consider consolidating.
741-768: Test name claims more than assertions verify.
TestSpinnerViews_ContainResetLineonly assertsNotEmpty, not that reset line escape sequences are present. Either rename toTestSpinnerViews_NotEmptyor assert on the actual escape sequence.🔎 Suggested fix to verify reset line
func TestSpinnerViews_ContainResetLine(t *testing.T) { + resetLine := "\r\033[K" t.Run("spinnerModel in-progress view has reset line", func(t *testing.T) { model := newSpinnerModel("test", "done") view := model.View() - // View should have the escape reset line sequence. - assert.NotEmpty(t, view) + assert.Contains(t, view, resetLine) }) // ... apply similarly to other subtests }
📜 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 (8)
cmd/list/affected.go(1 hunks)cmd/list/affected_test.go(1 hunks)internal/exec/describe_affected_helpers.go(3 hunks)internal/exec/terraform_output_utils.go(3 hunks)pkg/list/extract/affected.go(1 hunks)pkg/list/extract/affected_test.go(1 hunks)pkg/list/list_affected.go(1 hunks)pkg/ui/spinner/spinner_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/exec/terraform_output_utils.go
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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: NEVER use fmt.Fprintf(os.Stdout/Stderr) or fmt.Println(); use data.* or ui.* functions instead
All comments must end with periods (enforced by godot linter)
Organize imports in 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 for performance tracking; use nil if no atmosConfig param
All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Keep files small...
Files:
pkg/list/extract/affected_test.gocmd/list/affected_test.gopkg/list/list_affected.gopkg/ui/spinner/spinner_test.gocmd/list/affected.gopkg/list/extract/affected.gointernal/exec/describe_affected_helpers.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: Prefer unit tests with mocks over integration tests; use interfaces + dependency injection for testability; generate mocks with go.uber.org/mock/mockgen; use table-driven tests; target >80% coverage
Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Files:
pkg/list/extract/affected_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
cmd/**/*.go: Commands MUST use flags.NewStandardParser() for command-specific flags; NEVER call viper.BindEnv() or viper.BindPFlag() directly (Forbidigo enforces this); see cmd/version/version.go for reference
Embed CLI examples from cmd/markdown/*_usage.md using //go:embed; render with utils.PrintfMarkdown()
Files:
cmd/list/affected_test.gocmd/list/affected.go
cmd/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS use cmd.NewTestKit(t) for cmd tests to auto-clean RootCmd state (flags, args)
Files:
cmd/list/affected_test.go
🧠 Learnings (41)
📓 Common learnings
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: 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: 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: 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: 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: 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: 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).
📚 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:
pkg/list/extract/affected_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_test.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Applied to files:
pkg/list/extract/affected_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/list/extract/affected_test.gocmd/list/affected_test.gopkg/ui/spinner/spinner_test.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests; use interfaces + dependency injection for testability; generate mocks with go.uber.org/mock/mockgen; use table-driven tests; target >80% coverage
Applied to files:
pkg/list/extract/affected_test.gopkg/ui/spinner/spinner_test.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : Keep files small and focused (<600 lines); one cmd/impl per file; co-locate tests; never use //revive:disable:file-length-limit
Applied to files:
pkg/list/extract/affected_test.gocmd/list/affected_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 **/*.go : Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
Applied to files:
pkg/list/extract/affected_test.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
pkg/list/extract/affected_test.gocmd/list/affected_test.gopkg/list/list_affected.gocmd/list/affected.gopkg/list/extract/affected.gointernal/exec/describe_affected_helpers.go
📚 Learning: 2025-12-21T04:10:29.030Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration.
Applied to files:
pkg/list/extract/affected_test.gocmd/list/affected_test.gopkg/list/list_affected.gopkg/ui/spinner/spinner_test.gocmd/list/affected.gopkg/list/extract/affected.gointernal/exec/describe_affected_helpers.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/list/affected_test.gopkg/list/list_affected.gocmd/list/affected.go
📚 Learning: 2025-12-10T18:32:43.260Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:43.260Z
Learning: In cmd subpackages, tests should avoid using NewTestKit(t) unless tests actually interact with RootCmd (e.g., execute commands via RootCmd or modify RootCmd state). For structural tests that only verify command structure/flags without touching RootCmd, TestKit cleanup is unnecessary.
Applied to files:
cmd/list/affected_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
cmd/list/affected_test.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Applied to files:
cmd/list/affected_test.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/list/affected_test.gopkg/list/list_affected.gocmd/list/affected.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/list/affected_test.gocmd/list/affected.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/list/affected_test.gopkg/list/list_affected.go
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
cmd/list/affected_test.gocmd/list/affected.go
📚 Learning: 2025-06-07T19:28:21.289Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1266
File: cmd/describe_affected.go:0-0
Timestamp: 2025-06-07T19:28:21.289Z
Learning: In the Atmos codebase, using panic for unsupported flag types in flag processing functions like setDescribeAffectedFlagValueInCliArgs is the expected behavior rather than returning errors. This pattern is preferred for developer errors when unsupported types are added to the flagsKeyValue map.
Applied to files:
cmd/list/affected_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 meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Applied to files:
pkg/list/list_affected.gocmd/list/affected.go
📚 Learning: 2025-09-25T01:02:48.697Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/manager.go:304-312
Timestamp: 2025-09-25T01:02:48.697Z
Learning: The auth manager in pkg/auth/manager.go should remain cloud-agnostic and not contain AWS-specific logic or references to specific cloud providers. Keep the manager generic and extensible.
Applied to files:
pkg/list/list_affected.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:
pkg/list/list_affected.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:
pkg/list/list_affected.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:
pkg/list/list_affected.gocmd/list/affected.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:
pkg/list/list_affected.gocmd/list/affected.go
📚 Learning: 2025-12-13T06:10:13.688Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:13.688Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Applied to files:
pkg/list/list_affected.go
📚 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:
pkg/list/list_affected.go
📚 Learning: 2025-12-13T06:07:37.766Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Applied to files:
pkg/list/list_affected.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:
pkg/list/list_affected.go
📚 Learning: 2025-12-17T20:55:47.884Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1874
File: internal/exec/describe_affected_utils_test.go:436-468
Timestamp: 2025-12-17T20:55:47.884Z
Learning: In the Atmos codebase, there are two different paths for the `locked` flag: (1) filtering logic in `internal/exec/component_utils.go` (`isComponentLocked()`) reads from `componentSection["metadata"]["locked"]` to determine which components to include/exclude, and (2) extraction/rendering logic in `pkg/list/extract/affected.go` reads from `settings.metadata.locked` to display the locked status in output. Tests for filtering behavior should use `metadata.locked`.
Applied to files:
pkg/list/list_affected.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
pkg/list/list_affected.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : All comments must end with periods (enforced by godot linter)
Applied to files:
pkg/ui/spinner/spinner_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 **/*.go : Document complex logic with inline comments in Go code
Applied to files:
pkg/ui/spinner/spinner_test.go
📚 Learning: 2025-11-30T04:16:24.155Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:34-48
Timestamp: 2025-11-30T04:16:24.155Z
Learning: In the cloudposse/atmos repository, the `defer perf.Track()` guideline applies to functions that perform meaningful work (I/O, computation, external calls), but explicitly excludes trivial accessors/mutators (e.g., simple getters, setters with single integer increments, string joins, or map appends) where the tracking overhead would exceed the actual method cost and provide no actionable performance data. Hot-path methods called in tight loops should especially avoid perf.Track() if they perform only trivial operations.
Applied to files:
cmd/list/affected.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
cmd/list/affected.go
📚 Learning: 2025-11-30T04:16:01.899Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:50-59
Timestamp: 2025-11-30T04:16:01.899Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` should NOT be added to trivial O(1) getter methods that only return field references or check map lengths (e.g., `GetDeferredValues()`, `HasDeferredValues()`). The guideline to add perf tracking to "all public functions" applies to functions that do meaningful work (I/O, computation, external calls), not to trivial accessors where the tracking overhead would exceed the operation time and pollute performance reports.
Applied to files:
cmd/list/affected.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions for performance tracking; use nil if no atmosConfig param
Applied to files:
cmd/list/affected.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:
cmd/list/affected.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/list/affected.go
📚 Learning: 2025-12-13T03:21:35.786Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:35.786Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/list/affected.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/list/affected.go
📚 Learning: 2025-12-13T06:10:25.156Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: internal/exec/workflow_utils.go:0-0
Timestamp: 2025-12-13T06:10:25.156Z
Learning: Atmos workflows: In internal/exec/workflow_utils.go ExecuteWorkflow, non-identity steps intentionally use baseWorkflowEnv, which is constructed from the parent environment with PATH modifications for the toolchain. Avoid appending os.Environ() again; prefer documenting this behavior and testing that standard environment variables are preserved.
Applied to files:
internal/exec/describe_affected_helpers.go
🧬 Code graph analysis (5)
pkg/list/extract/affected_test.go (1)
pkg/schema/schema.go (1)
Dependent(898-913)
pkg/list/list_affected.go (6)
pkg/config/config.go (1)
InitCliConfig(28-67)pkg/list/extract/affected.go (1)
Affected(16-33)pkg/list/column/column.go (1)
NewSelector(52-79)pkg/list/renderer/renderer.go (1)
New(31-46)internal/exec/describe_affected_helpers.go (3)
ExecuteDescribeAffectedWithTargetRepoPath(275-335)ExecuteDescribeAffectedWithTargetRefClone(28-181)ExecuteDescribeAffectedWithTargetRefCheckout(187-265)pkg/list/sort/sort.go (4)
String(27-27)Sorter(35-39)ParseSortSpec(195-237)NewSorter(48-54)
pkg/ui/spinner/spinner_test.go (3)
cmd/cmd_utils.go (1)
Contains(1257-1264)pkg/ui/spinner/spinner.go (2)
ExecWithSpinnerDynamic(134-181)ExecWithSpinner(22-64)pkg/ui/formatter.go (1)
Success(193-200)
cmd/list/affected.go (4)
pkg/flags/standard_parser.go (1)
StandardParser(18-22)cmd/list/flag_wrappers.go (2)
NewListParser(414-425)WithFormatFlag(32-40)internal/exec/cli_utils.go (1)
ProcessCommandLineArgs(80-201)pkg/list/list_affected.go (2)
ExecuteListAffectedCmd(75-125)AffectedCommandOptions(46-72)
pkg/list/extract/affected.go (2)
pkg/perf/perf.go (1)
Track(121-138)pkg/schema/schema.go (1)
Dependent(898-913)
⏰ 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). (4)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (39)
internal/exec/describe_affected_helpers.go (3)
211-223: Clean target commit determination logic.The approach properly prioritizes SHA over ref and defaults to origin/HEAD when both are empty. The logging is clear and matches the commit type being used.
225-232: Solid worktree lifecycle management.Creating the worktree and using defer for cleanup ensures proper resource management even when errors occur downstream.
226-226: All worktree helper functions properly implemented and tested.The functions CreateWorktree, OpenWorktreeAwareRepo, RemoveWorktree, and GetWorktreeParentDir are well-implemented with comprehensive edge case handling. CreateWorktree properly returns errors for invalid refs and empty repos; OpenWorktreeAwareRepo uses EnableDotGitCommonDir for worktree support; RemoveWorktree gracefully handles missing worktrees with logging; GetWorktreeParentDir correctly extracts parent paths with cross-platform support. Tests cover critical scenarios and error cases.
cmd/list/affected.go (5)
1-12: Clean import organization.Imports are properly grouped: stdlib (none needed here), 3rd-party (cobra, viper), then Atmos packages. Good adherence to project conventions.
16-42: Well-structured options type.The
AffectedOptionsstruct logically groups related flags (output, git, content, processing). Embeddingglobal.Flagsis a clean pattern for shared options.
44-84: Command wiring looks solid.The
RunEfunction follows the expected pattern: check config, bind flags, populate options from Viper, delegate to execution. Usingcobra.NoArgsis correct since this command takes no positional arguments.
86-114: Parser initialization ininit()is well-composed.Using
NewListParserwith granularWithXFlagwrappers keeps the flag definitions modular. ThepaniconBindToViperfailure is acceptable for init-time configuration errors.
116-148: Execution function properly wired.Performance tracking is in place. The mapping from
AffectedOptionstoAffectedCommandOptionsis complete. TheFilterSpec: ""placeholder with a comment is a reasonable approach for future extensibility.pkg/list/extract/affected_test.go (5)
1-17: Good baseline tests for empty inputs.Testing both
niland empty slice ensures the function handles edge cases gracefully.
245-301: Table-driven test for status_text states.This is exactly the right approach - covers all four state combinations (enabled/locked matrix) in a single test with clear expectations.
390-428: Depth limit test validates recursion guard.Testing against
maxDependentDepthdirectly keeps the test in sync with implementation. The chain-building logic at line 403 cycles through 'a'-'z' which is fine for test identifiers.
353-388: Comprehensive field presence validation.Checking all expected fields exist in the result map is a good contract test that will catch regressions if fields are accidentally removed.
482-513: Multiple affected items with dependents ordering test.Validates that flattening produces the expected interleaved order (affected1, its deps, affected2, its deps). Good for catching ordering regressions.
pkg/list/extract/affected.go (5)
10-12: Reasonable recursion guard.
maxDependentDepth = 100is a sensible limit that prevents stack overflow while allowing deep dependency chains in practice.
14-33: Clean transformation logic.The
Affectedfunction is straightforward: iterate, transform, optionally flatten. Pre-allocating the result slice withlen(affected)capacity is a good optimization for the non-flattening case.
35-72:affectedToMapcovers all relevant fields.The map includes component metadata, status indicators, and dependent tracking. Having both
status(colored indicator) andstatus_text(semantic text) supports different output formats well.
74-93: Recursive flattening with depth check.The depth guard at line 86 (
depth < maxDependentDepth) correctly prevents infinite recursion. The recursion processes each level before moving to the next dependent.
132-162: Sensible default values for enabled/locked.Defaulting to
enabled=true, locked=falsewhen settings are absent matches the expected behavior for components without explicit metadata.pkg/list/list_affected.go (7)
25-43: Format-aware column configurations.Having separate
tableAffectedColumns(with colored status dot) anddataAffectedColumns(with semantic status_text) is a clean way to handle interactive vs. machine-readable output.
74-125: Solid execution flow with spinner feedback.The dynamic spinner message showing compared refs (
Compared \base`...`head``) provides helpful context to users. Early return with success message when no components are affected is clean UX.
192-250: Clear separation of git comparison strategies.The switch statement cleanly routes to the appropriate
DescribeAffectedvariant based on options. Each case consistently wraps the result inaffectedLogicResult.
252-265: Graceful handling of detached HEAD.The docstring and implementation correctly handle
nillocalRepoHead by leavingLocalRefempty, which the spinner handles with a generic "Compared branches" message.
302-337: Graceful fallback for column parsing.If parsing fails or produces no columns, falling back to
tableAffectedColumnsensures the command still works. Silently skipping malformed specs (lines 315-317, 322-324) could use a warning log, but the current behavior is acceptable.
339-366: Filter parsing is minimal but extensible.Single
field:valueorfield=valuefilter is a good starting point. The comment at line 351 notes future extension is planned.
368-392: Intuitive default sorting.Sorting by Stack then Component ascending makes the output predictable and grouped logically. Only applying this when both columns exist prevents runtime errors.
cmd/list/affected_test.go (8)
13-30: Good command structure tests.Testing
Use,Short,Long,RunE, andArgsvalidates the command is properly configured without needing to execute it.
32-69: Options struct field tests.While these tests primarily verify struct field assignment, they serve as documentation for all available options and their types.
93-157: Table-driven git options tests.Good use of table-driven tests to cover different git option combinations (ref only, SHA only, repo path, clone with SSH).
220-249: Comprehensive flag registration checks.Verifying all expected flags are registered catches issues where flags might be accidentally removed or renamed.
251-317: Flag/Viper precedence tests.Testing that flags take precedence over Viper values is important for ensuring the expected CLI behavior. The manual simulation of precedence logic is a reasonable approach.
579-618: Flag type contract tests.Verifying flag types (string, bool, stringSlice) ensures the API contract is maintained. This catches accidental type changes.
620-636: Registration verification.Checking
affectedis a subcommand oflistand thataffectedParseris initialized confirms the command wiring is complete.
638-683: Partial options tests are documentation-focused.These tests verify that options can be partially populated, which is more about documenting valid usage patterns than testing behavior. Acceptable for coverage.
pkg/ui/spinner/spinner_test.go (6)
1-11: LGTM!Imports are properly organized with stdlib first, then third-party packages.
169-293: Solid coverage fordynamicSpinnerModel.The tests properly verify constructor behavior, state transitions via Update, and View rendering for all key states (in-progress, completed with dynamic message, fallback, error).
295-334: Acceptable pragmatic approach for non-TTY tests.The
_ = errpattern with explanatory comments acknowledges the UI formatter dependency. Consider mocking the formatter in future to enable proper error assertions.
336-473: Good coverage formanualSpinnerModel.Tests properly verify the success/error toggle behavior and the line-clearing case when done with no message.
475-529: Public API tests cover the essential non-TTY behavior.Idempotence tests for
Stop,Success, andErrorare valuable for ensuring robustness.
834-865: Good idempotency coverage.Testing that
Stop,Success, andErrorcan be called multiple times or in mixed order without panicking is valuable for robustness.
|
These changes were released in v1.203.0-test.2. |
what
atmos list affectedcommand to compare affected components between branches-Cflag by correctly computing relative paths from git rootwhy
The original describe affected implementation had a bug where components appeared affected with reason
stack.metadataeven when there were zero differences between branches when using the-Cflag. This was caused by incorrect path calculation and cache contamination. The new list affected command provides a cleaner interface while fixing these issues. Additionally, the spinner implementation was duplicated across multiple files with formatting issues, so we centralized it in pkg/ui/spinner for better maintainability and proper markdown rendering support.references
-CflagSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.