feat: Create pkg/runner with unified task execution#1901
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
📝 WalkthroughWalkthroughReplaces Command.Steps from []string to structured Tasks, adds a runner (CommandRunner, Run/RunAll) for shell/atmos tasks with timeout/dir/stack handling, updates template rendering to pass each step's Command string, and adds tests, mocks, config decode hooks, and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Runner
participant CmdRunner as CommandRunner
participant Shell
participant Atmos
Caller->>Runner: Run(ctx, task, cmdRunner, opts)
Runner->>Runner: apply task.Timeout -> context with deadline
Runner->>Runner: resolve working dir (task.WorkingDirectory or opts.Dir)
alt type == "shell" or empty
Runner->>CmdRunner: RunShell(ctx, command, name, dir, env, dryRun)
CmdRunner->>Shell: execute shell command
Shell-->>CmdRunner: result/error
CmdRunner-->>Runner: result/error
else type == "atmos"
Runner->>Runner: parse task.Command -> args (shell.Fields or fallback)
Runner->>Runner: apply stack override (opts.Stack or task.Stack)
Runner->>Runner: appendStackArg(args, stack)
Runner->>CmdRunner: RunAtmos(ctx, AtmosExecParams{Args, Dir, Env, DryRun, Config})
CmdRunner->>Atmos: execute atmos with args
Atmos-->>CmdRunner: result/error
CmdRunner-->>Runner: result/error
else
Runner-->>Caller: ErrUnknownTaskType
end
Runner-->>Caller: result/error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/runner/runner.go (1)
109-137: Atmos task execution handles edge cases well.The fallback from
shell.Fieldstostrings.Fieldson parse error is defensive. Stack override logic (opts.Stacktakes precedence) is clear.One consideration: if
shell.Fieldsfails, it might be worth logging the parse error for debugging. Currently it silently falls back.Optional: Log shell.Fields parse failure
args, parseErr := shell.Fields(task.Command, nil) if parseErr != nil { - // Fall back to simple split if shell parsing fails. + // Fall back to simple split if shell parsing fails (e.g., unclosed quotes). + // Consider adding debug logging here if issues arise. args = strings.Fields(task.Command) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
cmd/cmd_utils.go(1 hunks)cmd/cmd_utils_test.go(1 hunks)cmd/custom_command_integration_test.go(8 hunks)pkg/runner/mock_runner_test.go(1 hunks)pkg/runner/runner.go(1 hunks)pkg/runner/runner_test.go(1 hunks)pkg/runner/task.go(1 hunks)pkg/runner/task_test.go(1 hunks)pkg/schema/command.go(2 hunks)pkg/schema/task.go(1 hunks)
🧰 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/runner/task_test.gocmd/custom_command_integration_test.gopkg/schema/task.gopkg/runner/runner_test.gocmd/cmd_utils.gopkg/runner/task.gopkg/runner/mock_runner_test.gopkg/schema/command.gocmd/cmd_utils_test.gopkg/runner/runner.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/runner/task_test.gocmd/custom_command_integration_test.gopkg/runner/runner_test.gopkg/runner/mock_runner_test.gocmd/cmd_utils_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
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/custom_command_integration_test.gocmd/cmd_utils.gocmd/cmd_utils_test.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/custom_command_integration_test.gocmd/cmd_utils_test.go
🧠 Learnings (26)
📚 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/runner/task_test.gopkg/runner/runner_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/runner/task_test.gocmd/custom_command_integration_test.gopkg/runner/runner_test.gopkg/runner/mock_runner_test.gocmd/cmd_utils_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/runner/task_test.gocmd/custom_command_integration_test.gopkg/runner/runner_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/runner/task_test.gopkg/runner/runner_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 .github/workflows/*.{yml,yaml} : Configure CI to run unit tests, integration tests, golangci-lint, and coverage reporting on all pull requests
Applied to files:
pkg/runner/task_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/runner/task_test.gopkg/runner/runner_test.gopkg/runner/mock_runner_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Ensure all tests pass, verify code coverage meets targets, run golangci-lint and fix any issues, and update documentation before submitting pull requests
Applied to files:
pkg/runner/task_test.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:
pkg/runner/task_test.gocmd/custom_command_integration_test.gocmd/cmd_utils.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
Applied to files:
cmd/custom_command_integration_test.gocmd/cmd_utils.gopkg/schema/command.gocmd/cmd_utils_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Applied to files:
cmd/custom_command_integration_test.gocmd/cmd_utils.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/custom_command_integration_test.gocmd/cmd_utils_test.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
cmd/custom_command_integration_test.gopkg/runner/runner_test.gocmd/cmd_utils.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
cmd/custom_command_integration_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
cmd/custom_command_integration_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/custom_command_integration_test.gocmd/cmd_utils.gocmd/cmd_utils_test.go
📚 Learning: 2025-12-10T18:32:51.237Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:51.237Z
Learning: In cmd subpackages (e.g., cmd/terraform/backend/), tests cannot use cmd.NewTestKit(t) due to Go's test visibility rules (NewTestKit is in a parent package test file). These tests only need TestKit if they execute commands through RootCmd or modify RootCmd state. Structural tests that only verify command structure/flags without touching RootCmd don't require TestKit cleanup.
Applied to files:
pkg/runner/runner_test.go
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1261
File: internal/exec/utils.go:639-640
Timestamp: 2025-05-22T15:42:10.906Z
Learning: In the Atmos codebase, when appending slices with `args := append(configAndStacksInfo.CliArgs, configAndStacksInfo.AdditionalArgsAndFlags...)`, it's intentional that the result is not stored back in the original slice. This pattern is used when the merged result serves a different purpose than the original slices, such as when creating a filtered version for component section assignments.
Applied to files:
pkg/runner/runner_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:
pkg/runner/runner_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:
pkg/runner/runner_test.gocmd/cmd_utils.go
📚 Learning: 2025-01-25T03:49:03.951Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: internal/exec/template_utils.go:268-271
Timestamp: 2025-01-25T03:49:03.951Z
Learning: The `ProcessTmplWithDatasourcesGomplate` function in `internal/exec/template_utils.go` is used for documentation generation purposes, where simple environment variable handling is acceptable and thread-safety concerns are not critical.
Applied to files:
cmd/cmd_utils.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Applied to files:
cmd/cmd_utils.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: New configs support Go templating with FuncMap() from internal/exec/template_funcs.go
Applied to files:
cmd/cmd_utils.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: Define interfaces for all major functionality and use dependency injection for testability; generate mocks with go.uber.org/mock/mockgen
Applied to files:
pkg/runner/mock_runner_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/runner/mock_runner_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 : Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Applied to files:
pkg/runner/mock_runner_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:
pkg/runner/runner.go
🧬 Code graph analysis (8)
pkg/runner/task_test.go (3)
pkg/schema/task.go (2)
Task(22-39)TaskFromWorkflowStep(113-124)pkg/schema/schema.go (1)
RetryConfig(784-792)pkg/schema/workflow.go (1)
WorkflowStep(7-15)
cmd/custom_command_integration_test.go (2)
pkg/schema/task.go (2)
Tasks(64-64)Task(22-39)pkg/schema/command.go (1)
Command(5-21)
cmd/cmd_utils.go (2)
internal/exec/template_utils.go (1)
ProcessTmpl(53-97)pkg/schema/command.go (1)
Command(5-21)
pkg/runner/task.go (1)
pkg/schema/task.go (2)
Task(22-39)Tasks(64-64)
pkg/runner/mock_runner_test.go (1)
pkg/runner/runner.go (1)
AtmosExecParams(44-55)
pkg/schema/command.go (2)
pkg/runner/task.go (1)
Tasks(13-13)pkg/schema/task.go (1)
Tasks(64-64)
cmd/cmd_utils_test.go (3)
pkg/runner/task.go (1)
Tasks(13-13)pkg/schema/task.go (1)
Tasks(64-64)pkg/schema/command.go (1)
Command(5-21)
pkg/runner/runner.go (4)
pkg/schema/schema.go (1)
AtmosConfiguration(54-99)pkg/runner/task.go (1)
Task(9-9)pkg/schema/task.go (1)
Task(22-39)pkg/perf/perf.go (1)
Track(121-138)
⏰ 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 (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (21)
pkg/schema/task.go (5)
11-17: LGTM!Clean sentinel error definitions following Go conventions.
19-39: LGTM!Solid struct design with proper tags and documentation. The
time.Durationfor Timeout will parse cleanly from YAML strings like "30s".
66-97: LGTM!Robust unmarshalling logic. Handles both simple strings and structured mappings cleanly, defaults Type to "shell" consistently, and provides clear error context with indices.
99-110: LGTM!Clean conversion maintaining field parity with WorkflowStep.
112-124: LGTM!Good documentation noting the Timeout field difference between types.
pkg/runner/task.go (1)
1-13: LGTM!Clean type aliases. This approach keeps the runner package API self-contained while reusing schema types.
cmd/cmd_utils.go (2)
602-604: LGTM!Correctly accesses
step.Commandnow that Steps is aTasksslice. Template processing continues to work with the command string.
490-613: Timeout field is defined but not enforced here.The
Task.Timeoutfield is available but not used during custom command step execution. If timeout support is expected for custom commands in this PR, you'll need to wrap the shell execution with context timeout handling.If this is intentional (deferring timeout support for custom commands), consider documenting that limitation.
pkg/schema/command.go (1)
13-17: LGTM!Clear documentation of the mixed syntax support. The
Taskstype with custom unmarshalling maintains YAML backward compatibility while enabling structured steps.cmd/custom_command_integration_test.go (3)
18-25: LGTM!Clean test helper. Mirrors the unmarshalling behavior where simple strings become shell-type tasks.
63-63: LGTM!Test correctly adapted to use the helper function.
393-401: LGTM!Multi-step test with template patterns correctly uses the helper. Good coverage of various Go template patterns with boolean flags.
pkg/runner/task_test.go (1)
1-237: Solid test coverage for Task YAML unmarshalling.Good use of separate test functions covering simple strings, structured syntax, mixed formats, defaults, optional fields (retry, working_directory, identity), empty lists, error cases, and conversion helpers. The tests are readable and verify behavior without over-testing implementation details.
One minor consideration: a table-driven approach could consolidate some similar tests (e.g., the field-specific tests like
WithRetry,WithWorkingDirectory,WithIdentity), but the current structure is clear and maintainable. Based on learnings, table-driven tests are preferred for testing multiple scenarios in Go.cmd/cmd_utils_test.go (1)
967-979: Correctly updated for new Tasks type.The test input now constructs
schema.Taskswith properTaskstructs, and the assertion correctly verifies deep-copy behavior by comparingCommandfields. Clean adaptation to the schema change.pkg/runner/runner_test.go (1)
1-306: Comprehensive runner tests with proper mock usage.Strong coverage across all execution paths. Good use of
gomock.InOrderfor sequential verification inTestRunAll_Success. The timeout test properly validates context deadline propagation. Error handling and fail-fast behavior are well-tested.The tests follow the coding guidelines: using gomock for mocks, testing behavior rather than implementation, and verifying error propagation with
errors.Is()where appropriate.pkg/runner/mock_runner_test.go (1)
1-69: Generated mock follows project conventions.Properly generated using
go.uber.org/mock/mockgenas specified in coding guidelines. The//go:generatedirective inrunner.go(line 25) ensures regeneration stays in sync. No manual edits needed.pkg/runner/runner.go (5)
15-19: Good sentinel error definition.Using a package-level sentinel error enables callers to use
errors.Is()for error checking, following the coding guidelines.
21-41: Clean interface design with proper documentation.The
CommandRunnerinterface is well-documented with parameter descriptions. The//go:generatedirective follows project conventions for mock generation.
71-107: Solid execution logic with proper timeout handling.Good implementation:
- Timeout via
context.WithTimeoutwith properdefer cancel()- Working directory precedence: task > opts > default "."
- Default type to "shell" aligns with schema documentation
- Error wrapping uses
%wforErrUnknownTaskType
139-155: Correct handling of--separator.The
appendStackArgfunction correctly inserts-s <stack>before any--separator, preserving pass-through arguments. Edge cases are well-handled.
157-176: RunAll provides clear fail-fast semantics.The error message includes task index and name for easy debugging. The
taskNamehelper generates a fallback label whenNameis empty.
03fd731 to
8a199e4
Compare
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (13)
cmd/cmd_utils.go(1 hunks)cmd/cmd_utils_test.go(1 hunks)cmd/custom_command_integration_test.go(8 hunks)pkg/config/const.go(1 hunks)pkg/config/load.go(5 hunks)pkg/config/load_config_args.go(1 hunks)pkg/runner/mock_runner_test.go(1 hunks)pkg/runner/runner.go(1 hunks)pkg/runner/runner_test.go(1 hunks)pkg/runner/task.go(1 hunks)pkg/runner/task_test.go(1 hunks)pkg/schema/command.go(2 hunks)pkg/schema/task.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- cmd/cmd_utils.go
- cmd/custom_command_integration_test.go
- cmd/cmd_utils_test.go
- pkg/runner/mock_runner_test.go
- pkg/schema/task.go
- pkg/runner/runner_test.go
- pkg/runner/task_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/runner/task.gopkg/config/const.gopkg/config/load_config_args.gopkg/config/load.gopkg/runner/runner.gopkg/schema/command.go
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
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
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
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.
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.
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Define interfaces for all major functionality and use dependency injection for testability; generate mocks with go.uber.org/mock/mockgen
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to cmd/**/*.go : Embed CLI examples from cmd/markdown/*_usage.md using //go:embed; render with utils.PrintfMarkdown()
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
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:51.237Z
Learning: In cmd subpackages (e.g., cmd/terraform/backend/), tests cannot use cmd.NewTestKit(t) due to Go's test visibility rules (NewTestKit is in a parent package test file). These tests only need TestKit if they execute commands through RootCmd or modify RootCmd state. Structural tests that only verify command structure/flags without touching RootCmd don't require TestKit cleanup.
📚 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/config/const.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:
pkg/config/const.gopkg/config/load_config_args.gopkg/config/load.go
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1261
File: internal/exec/utils.go:639-640
Timestamp: 2025-05-22T15:42:10.906Z
Learning: In the Atmos codebase, when appending slices with `args := append(configAndStacksInfo.CliArgs, configAndStacksInfo.AdditionalArgsAndFlags...)`, it's intentional that the result is not stored back in the original slice. This pattern is used when the merged result serves a different purpose than the original slices, such as when creating a filtered version for component section assignments.
Applied to files:
pkg/config/const.gopkg/config/load.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/config/load_config_args.gopkg/config/load.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:
pkg/config/load_config_args.gopkg/config/load.gopkg/runner/runner.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
pkg/config/load_config_args.gopkg/config/load.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:
pkg/config/load_config_args.gopkg/config/load.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/config/load_config_args.gopkg/config/load.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:
pkg/config/load_config_args.go
📚 Learning: 2025-12-13T04:37:25.223Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/root.go:0-0
Timestamp: 2025-12-13T04:37:25.223Z
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:
pkg/config/load_config_args.gopkg/config/load.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
pkg/config/load_config_args.gopkg/config/load.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:
pkg/config/load_config_args.gopkg/config/load.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:
pkg/config/load_config_args.gopkg/config/load.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
pkg/config/load_config_args.gopkg/config/load.go
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Applied to files:
pkg/config/load_config_args.gopkg/config/load.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:
pkg/config/load.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 : Use ATMOS_ prefix for environment variables with viper.BindEnv("ATMOS_VAR", "ATMOS_VAR", "FALLBACK")
Applied to files:
pkg/config/load.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 Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
pkg/config/load.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
Applied to files:
pkg/config/load.gopkg/schema/command.go
📚 Learning: 2025-04-24T01:06:15.259Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1202
File: pkg/config/process_yaml.go:0-0
Timestamp: 2025-04-24T01:06:15.259Z
Learning: When processing YAML structures with gopkg.in/yaml.v3, the Value field of a Node is only meant for scalar values. Complex data types like maps and arrays are represented through child nodes in the YAML tree structure.
Applied to files:
pkg/config/load.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 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
Applied to files:
pkg/config/load.go
📚 Learning: 2025-04-03T10:57:04.602Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1091
File: pkg/config/load_config_args.go:61-82
Timestamp: 2025-04-03T10:57:04.602Z
Learning: In the Atmos configuration system, errors in `mergeDefaultImports` and `mergeImports` functions are intentionally only logged as debug messages without returning errors, as they are considered non-critical. Only errors in critical functions like `mergeConfigFile` are returned to halt execution.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-03-17T18:41:08.831Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/imports.go:68-75
Timestamp: 2025-03-17T18:41:08.831Z
Learning: In the Atmos configuration import system, errors during config file merging are logged at debug level and the process continues with other imports rather than failing completely, prioritizing resilience over strict correctness.
Applied to files:
pkg/config/load.go
🧬 Code graph analysis (4)
pkg/runner/task.go (1)
pkg/schema/task.go (2)
Task(32-49)Tasks(74-74)
pkg/config/load.go (2)
pkg/config/const.go (1)
SliceSeparator(154-154)pkg/schema/task.go (1)
TasksDecodeHook(139-159)
pkg/runner/runner.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(54-99)pkg/schema/task.go (4)
Task(32-49)TaskTypeShell(16-16)TaskTypeAtmos(18-18)Tasks(74-74)
pkg/schema/command.go (2)
pkg/runner/task.go (1)
Tasks(13-13)pkg/schema/task.go (1)
Tasks(74-74)
⏰ 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 (windows)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Summary
🔇 Additional comments (15)
pkg/config/const.go (1)
152-154: LGTM – Clean constant addition.The
SliceSeparatorconstant is well-documented and follows Go conventions. It's properly integrated with the new decode hook for splitting comma-separated strings.pkg/config/load.go (3)
14-14: LGTM – Import added for decode hooks.The mapstructure import is necessary for composing custom decode hooks in
atmosDecodeHook().
298-298: LGTM – Consistent decode hook application.The
atmosDecodeHook()is applied to all three unmarshaling paths (temporary config for profiles, main config, and imports), ensuring consistent YAML decoding behavior across all configuration sources.Also applies to: 319-319, 1052-1052
1295-1305: LGTM – Well-documented decode hook composition.The
atmosDecodeHookproperly composes standard mapstructure hooks with the customTasksDecodeHookfor flexible task parsing. The documentation clearly explains the hook's purpose and composition.pkg/config/load_config_args.go (1)
48-48: LGTM – Decode hook applied consistently.The
atmosDecodeHook()is properly applied here to ensure CLI-arg-loaded configurations benefit from the same enhanced decoding as other config sources.pkg/runner/task.go (1)
1-13: LGTM – Clean type aliasing for package boundaries.The type aliases properly expose
schema.Taskandschema.Tasksin the runner package, maintaining clean package boundaries while enabling shared type usage. Documentation clearly explains their purpose.pkg/schema/command.go (2)
3-4: LGTM – Comment clarification.The updated comment clearly identifies this as custom CLI command schema.
13-20: LGTM – Well-documented schema evolution.The
Stepsfield evolution from[]stringtoTasksis well-documented with clear examples of simple, structured, and mixed syntax. TheTasksDecodeHookensures backward compatibility with existing simple string steps while enabling new structured syntax.pkg/runner/runner.go (7)
15-19: LGTM – Sentinel error properly defined.The
ErrUnknownTaskTypesentinel error follows Go error handling best practices and enables callers to useerrors.Is()for error checking.
21-41: LGTM – Well-designed interface with mockgen directive.The
CommandRunnerinterface provides clean abstraction for testing, with comprehensive parameter documentation. The//go:generatedirective follows the coding guidelines for mock generation.
43-55: LGTM – Clean parameter struct.The
AtmosExecParamsstruct is well-documented and groups related parameters logically for atmos command execution.
74-107: LGTM – Solid task execution with timeout support.The
Runfunction properly handles timeout enforcement via context, working directory resolution, and task type dispatching. Performance tracking is correctly applied.
110-137: LGTM – Robust argument parsing with fallback.The
runAtmosTaskfunction usesshell.Fieldsfor proper quote handling with a sensible fallback tostrings.Fields. Stack override logic is correctly implemented.
140-155: LGTM – Correct handling of double-dash separator.The
appendStackArgfunction properly inserts-s <stack>before the--separator when present, maintaining correct argument order for commands that pass additional flags after the separator.
159-176: LGTM – Sequential task execution with clear error reporting.The
RunAllfunction executes tasks sequentially with proper error context including task index and name. ThetaskNamehelper provides fallback naming for unnamed tasks.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
website/blog/2025-12-20-unified-task-runner.mdx (2)
12-68: Add references section per PR template.The coding guidelines require blog posts to follow the PR template (what/why/references). Consider adding a references section mentioning DEV-3005 or linking to the PR/issue for traceability.
🔎 Suggested addition
Add a references section before the closing of the post:
This is an internal architectural improvement with full backward compatibility for existing custom command configurations. + +## References + +- Closes [DEV-3005](link-to-issue) - Extend Custom Commands for Workflow Compatible Syntax +- [PR #1901](https://github.com/cloudposse/atmos/pull/1901)Based on coding guidelines requiring PR template structure.
51-58: Consider emphasizing the workflow unification aspect.The PR summary indicates this provides "a shared execution layer for custom commands and workflows." While line 53 mentions this groundwork, you might want to clarify earlier that this unified execution model serves both custom commands and workflows to give readers the full picture.
📜 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)
website/blog/2025-12-20-unified-task-runner.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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-20-unified-task-runner.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-20-unified-task-runner.mdx
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/schema/task.go (1)
188-208: Consider adding slice hook for nested fields.The decoder in
decodeTaskFromMaponly includesStringToTimeDurationHookFunc. If any futureTaskfields need slice parsing (e.g., tags), you'd need to addStringToSliceHookFunchere too.🔎 Optional: Add slice hook for future-proofing
func decodeTaskFromMap(m map[string]any, index int) (Task, error) { var task Task decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ Result: &task, TagName: "mapstructure", WeaklyTypedInput: true, - DecodeHook: mapstructure.StringToTimeDurationHookFunc(), + DecodeHook: mapstructure.ComposeDecodeHookFunc( + mapstructure.StringToTimeDurationHookFunc(), + mapstructure.StringToSliceHookFunc(","), + ), })
📜 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 (14)
cmd/cmd_utils.go(1 hunks)cmd/cmd_utils_test.go(1 hunks)cmd/custom_command_integration_test.go(8 hunks)pkg/config/const.go(1 hunks)pkg/config/load.go(5 hunks)pkg/config/load_config_args.go(1 hunks)pkg/runner/mock_runner_test.go(1 hunks)pkg/runner/runner.go(1 hunks)pkg/runner/runner_test.go(1 hunks)pkg/runner/task.go(1 hunks)pkg/runner/task_test.go(1 hunks)pkg/schema/command.go(2 hunks)pkg/schema/task.go(1 hunks)website/blog/2025-12-20-unified-task-runner.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- pkg/runner/mock_runner_test.go
- pkg/runner/task.go
- cmd/custom_command_integration_test.go
- cmd/cmd_utils_test.go
- cmd/cmd_utils.go
- pkg/runner/task_test.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/config/const.gopkg/config/load.gopkg/runner/runner_test.gopkg/config/load_config_args.gopkg/schema/command.gopkg/runner/runner.gopkg/schema/task.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-20-unified-task-runner.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-20-unified-task-runner.mdx
**/*_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/runner/runner_test.go
🧠 Learnings (36)
📓 Common learnings
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
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1261
File: internal/exec/utils.go:639-640
Timestamp: 2025-05-22T15:42:10.906Z
Learning: In the Atmos codebase, when appending slices with `args := append(configAndStacksInfo.CliArgs, configAndStacksInfo.AdditionalArgsAndFlags...)`, it's intentional that the result is not stored back in the original slice. This pattern is used when the merged result serves a different purpose than the original slices, such as when creating a filtered version for component section assignments.
Applied to files:
pkg/config/const.gopkg/config/load.gopkg/runner/runner_test.go
📚 Learning: 2025-09-30T19:03:50.738Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 1560
File: pkg/utils/string_utils.go:43-64
Timestamp: 2025-09-30T19:03:50.738Z
Learning: In the Atmos codebase, YAML tags like !terraform.output rely on positional arguments, so the SplitStringByDelimiter function in pkg/utils/string_utils.go must preserve empty strings (even after trimming quotes) to maintain the correct number of positional arguments. Filtering out empty values after trimming would collapse the array and break these function calls.
Applied to files:
pkg/config/const.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/config/const.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:
pkg/config/const.gopkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.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:
pkg/config/load.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/config/load.gopkg/config/load_config_args.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:
pkg/config/load.gopkg/config/load_config_args.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 Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
pkg/config/load.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
Applied to files:
pkg/config/load.gopkg/schema/command.go
📚 Learning: 2025-04-24T01:06:15.259Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1202
File: pkg/config/process_yaml.go:0-0
Timestamp: 2025-04-24T01:06:15.259Z
Learning: When processing YAML structures with gopkg.in/yaml.v3, the Value field of a Node is only meant for scalar values. Complex data types like maps and arrays are represented through child nodes in the YAML tree structure.
Applied to files:
pkg/config/load.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 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
Applied to files:
pkg/config/load.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:
pkg/config/load.gopkg/config/load_config_args.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:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-12-13T04:37:25.223Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/root.go:0-0
Timestamp: 2025-12-13T04:37:25.223Z
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:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.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:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-04-03T10:57:04.602Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1091
File: pkg/config/load_config_args.go:61-82
Timestamp: 2025-04-03T10:57:04.602Z
Learning: In the Atmos configuration system, errors in `mergeDefaultImports` and `mergeImports` functions are intentionally only logged as debug messages without returning errors, as they are considered non-critical. Only errors in critical functions like `mergeConfigFile` are returned to halt execution.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-03-17T18:41:08.831Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/imports.go:68-75
Timestamp: 2025-03-17T18:41:08.831Z
Learning: In the Atmos configuration import system, errors during config file merging are logged at debug level and the process continues with other imports rather than failing completely, prioritizing resilience over strict correctness.
Applied to files:
pkg/config/load.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/runner/runner_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/runner/runner_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/runner/runner_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/runner/runner_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/runner/runner_test.go
📚 Learning: 2025-12-10T18:32:51.237Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:51.237Z
Learning: In cmd subpackages (e.g., cmd/terraform/backend/), tests cannot use cmd.NewTestKit(t) due to Go's test visibility rules (NewTestKit is in a parent package test file). These tests only need TestKit if they execute commands through RootCmd or modify RootCmd state. Structural tests that only verify command structure/flags without touching RootCmd don't require TestKit cleanup.
Applied to files:
pkg/runner/runner_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:
pkg/runner/runner_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:
pkg/runner/runner_test.gopkg/config/load_config_args.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/runner/runner_test.gopkg/schema/command.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
pkg/config/load_config_args.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:
pkg/schema/command.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/schema/command.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/runner/runner.go
🧬 Code graph analysis (4)
pkg/config/load.go (2)
pkg/config/const.go (1)
SliceSeparator(154-154)pkg/schema/task.go (1)
TasksDecodeHook(139-159)
pkg/runner/runner_test.go (3)
pkg/schema/task.go (2)
Task(32-49)Tasks(74-74)pkg/schema/command.go (1)
Command(5-21)pkg/runner/runner.go (5)
Options(58-69)Run(74-107)AtmosExecParams(44-55)ErrUnknownTaskType(18-18)RunAll(159-168)
pkg/schema/command.go (2)
pkg/runner/task.go (1)
Tasks(13-13)pkg/schema/task.go (1)
Tasks(74-74)
pkg/schema/task.go (5)
pkg/schema/command.go (1)
Command(5-21)pkg/schema/schema.go (1)
RetryConfig(784-792)pkg/runner/task.go (2)
Task(9-9)Tasks(13-13)pkg/utils/yaml_utils.go (1)
UnmarshalYAML(690-692)pkg/schema/workflow.go (1)
WorkflowStep(7-15)
⏰ 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). (1)
- GitHub Check: Summary
🔇 Additional comments (27)
pkg/config/const.go (1)
152-154: Clean addition.Centralizing the slice separator constant is solid. It's referenced by
atmosDecodeHook()inload.gofor consistent parsing.pkg/config/load.go (2)
1295-1305: Well-structured decode hook composition.The
atmosDecodeHook()properly combines duration parsing, slice parsing, and the newTasksDecodeHook. The composition ensures consistent behavior across all config unmarshal paths.
298-300: Consistent hook application.Good to see all three unmarshal points updated uniformly.
pkg/schema/task.go (4)
29-49: Solid unified Task type.Clean struct covering all execution metadata. The field alignment with
WorkflowStepenables seamless conversion. Timeout field adds per-step control thatWorkflowSteplacked.
76-107: Clean YAML unmarshaling.The switch on
node.Kindcovers both syntaxes elegantly. Default type assignment in both branches ensures consistency.
109-134: Conversion functions are straightforward.The comment on line 132 clarifies the
Timeoutomission. The shallow copy ofRetrypointer is appropriate for shared config scenarios.
136-159: Well-designed decode hook.The early returns for non-matching types are efficient. The hook correctly passes through data unchanged when not targeting
Tasks.pkg/config/load_config_args.go (1)
48-50: Consistent with main load path.The decode hook is now applied uniformly across CLI arg loading too. Good.
pkg/schema/command.go (1)
13-17: Good backward-compatible design.The
Taskstype with its customUnmarshalYAMLpreserves backward compatibility at the YAML layer while enabling structured syntax. TheTasksDecodeHookprovides the same flexibility for mapstructure unmarshaling. Inline docs clearly explain the three supported formats. Code usingCommand.Stepscorrectly accesses theTask.Commandfield.website/blog/2025-12-20-unified-task-runner.mdx (2)
1-10: LGTM!Front matter is properly structured with valid
coretag, author from authors.yml, and<!--truncate-->correctly placed after the intro paragraph. Good structure.
12-68: Clear documentation of the feature.Examples effectively demonstrate all three syntax patterns (simple, structured, mixed). The contributor section covers the key public API surface. Solid reference material.
pkg/runner/runner_test.go (7)
1-12: Imports well-organized.Three groups properly separated: stdlib, third-party, then atmos packages (none needed here). Good use of gomock per guidelines.
14-36: Solid shell task test.Correctly verifies that shell tasks pass command, name, and working directory to RunShell.
61-89: Good coverage of Atmos task execution.Validates command parsing into args and stack flag insertion. The use of
DoAndReturnfor detailed assertion is appropriate here.
164-180: Correct use of errors.Is for error checking.Per coding guidelines, using
assert.ErrorIsto check forErrUnknownTaskType. Good practice.
205-231: Timeout setup verified.Good test for confirming the context deadline is applied. Worth noting this doesn't test actual timeout cancellation behavior (where the runner would return
context.DeadlineExceeded), but the setup is correctly validated.
233-280: RunAll tests cover key scenarios.Sequential execution, early termination on error, and empty tasks edge case. The use of
gomock.InOrdercorrectly enforces execution order.
296-306: Good unit tests for helper function.Both separator scenarios covered. Simple and effective.
pkg/runner/runner.go (9)
1-13: Imports properly organized.Three groups: stdlib, third-party (mvdan.cc/sh), atmos packages. Follows guidelines.
15-19: Sentinel error correctly defined.
ErrUnknownTaskTypefollows the sentinel error pattern. Comment ends with period per godot linter.
21-41: Well-designed interface for testability.The
//go:generatedirective follows guidelines for mock generation. Detailed parameter docs make the contract clear.
43-69: Structs well-documented.Clear field descriptions. Past review comment about missing period on Line 63 has been addressed.
71-107: Run function implementation looks solid.Proper use of
defer perf.Track, context-based timeout with cancel, working directory precedence logic, and error wrapping with%w. Clean control flow.
109-137: Command parsing with shell.Fields is the right choice.Proper quote handling via
mvdan.cc/sh/v3/shellwith fallback tostrings.Fields. Stack precedence logic is clear.
139-155: Correct handling of -- separator.Stack arg is properly inserted before
--when present. Clean implementation.
157-168: RunAll correctly stops on first error.
perf.Trackpresent. Error message is helpful. Minor note: if task has no name, the error reads "task 1 (step2) failed" which shows index twice (0-based and 1-based). This is fine for debugging but slightly redundant.
170-176: Simple and effective helper.Returns human-readable step names with 1-based indexing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/config/load.go (1)
1295-1305: Consider adding performance tracking.The atmosDecodeHook() function is exported but lacks
defer perf.Track(atmosConfig, "config.atmosDecodeHook")(). Per coding guidelines, all public functions should include performance tracking.However, this function returns a config option (not performing heavy work itself), so tracking might be intentional omission. If this is a lightweight helper, document why tracking is skipped.
📋 Proposed addition if tracking is desired
func atmosDecodeHook() viper.DecoderConfigOption { + // Note: No perf tracking - this is a lightweight config option generator return viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( mapstructure.StringToTimeDurationHookFunc(), mapstructure.StringToSliceHookFunc(SliceSeparator), schema.TasksDecodeHook(), )) }
📜 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 (14)
cmd/cmd_utils.go(1 hunks)cmd/cmd_utils_test.go(1 hunks)cmd/custom_command_integration_test.go(8 hunks)pkg/config/const.go(1 hunks)pkg/config/load.go(5 hunks)pkg/config/load_config_args.go(1 hunks)pkg/runner/mock_runner_test.go(1 hunks)pkg/runner/runner.go(1 hunks)pkg/runner/runner_test.go(1 hunks)pkg/runner/task.go(1 hunks)pkg/runner/task_test.go(1 hunks)pkg/schema/command.go(2 hunks)pkg/schema/task.go(1 hunks)website/blog/2025-12-20-unified-task-runner.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- website/blog/2025-12-20-unified-task-runner.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/runner/runner.go
- pkg/runner/task_test.go
- cmd/cmd_utils_test.go
- pkg/runner/runner_test.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/config/const.gopkg/runner/mock_runner_test.gopkg/schema/command.gocmd/custom_command_integration_test.gopkg/config/load.gopkg/config/load_config_args.gopkg/runner/task.gopkg/schema/task.gocmd/cmd_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: 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/runner/mock_runner_test.gocmd/custom_command_integration_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/custom_command_integration_test.gocmd/cmd_utils.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/custom_command_integration_test.go
🧠 Learnings (38)
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1261
File: internal/exec/utils.go:639-640
Timestamp: 2025-05-22T15:42:10.906Z
Learning: In the Atmos codebase, when appending slices with `args := append(configAndStacksInfo.CliArgs, configAndStacksInfo.AdditionalArgsAndFlags...)`, it's intentional that the result is not stored back in the original slice. This pattern is used when the merged result serves a different purpose than the original slices, such as when creating a filtered version for component section assignments.
Applied to files:
pkg/config/const.gopkg/config/load.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/config/const.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:
pkg/config/const.gopkg/config/load.gopkg/config/load_config_args.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: Define interfaces for all major functionality and use dependency injection for testability; generate mocks with go.uber.org/mock/mockgen
Applied to files:
pkg/runner/mock_runner_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/runner/mock_runner_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/runner/mock_runner_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 : Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Applied to files:
pkg/runner/mock_runner_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/runner/mock_runner_test.gocmd/custom_command_integration_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:
pkg/schema/command.gocmd/custom_command_integration_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 : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
Applied to files:
pkg/schema/command.gocmd/custom_command_integration_test.gopkg/config/load.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/custom_command_integration_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/custom_command_integration_test.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
cmd/custom_command_integration_test.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:
cmd/custom_command_integration_test.gocmd/cmd_utils.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
cmd/custom_command_integration_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/custom_command_integration_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/custom_command_integration_test.gocmd/cmd_utils.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.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:
pkg/config/load.gopkg/config/load_config_args.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 : Use ATMOS_ prefix for environment variables with viper.BindEnv("ATMOS_VAR", "ATMOS_VAR", "FALLBACK")
Applied to files:
pkg/config/load.gopkg/config/load_config_args.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:
pkg/config/load.gopkg/config/load_config_args.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/config/load.gopkg/config/load_config_args.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 Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
pkg/config/load.go
📚 Learning: 2025-04-24T01:06:15.259Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1202
File: pkg/config/process_yaml.go:0-0
Timestamp: 2025-04-24T01:06:15.259Z
Learning: When processing YAML structures with gopkg.in/yaml.v3, the Value field of a Node is only meant for scalar values. Complex data types like maps and arrays are represented through child nodes in the YAML tree structure.
Applied to files:
pkg/config/load.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 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
Applied to files:
pkg/config/load.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:
pkg/config/load.gopkg/config/load_config_args.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:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-12-13T04:37:25.223Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/root.go:0-0
Timestamp: 2025-12-13T04:37:25.223Z
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:
pkg/config/load.gopkg/config/load_config_args.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:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Applied to files:
pkg/config/load.gopkg/config/load_config_args.go
📚 Learning: 2025-04-03T10:57:04.602Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1091
File: pkg/config/load_config_args.go:61-82
Timestamp: 2025-04-03T10:57:04.602Z
Learning: In the Atmos configuration system, errors in `mergeDefaultImports` and `mergeImports` functions are intentionally only logged as debug messages without returning errors, as they are considered non-critical. Only errors in critical functions like `mergeConfigFile` are returned to halt execution.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-03-17T18:41:08.831Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/imports.go:68-75
Timestamp: 2025-03-17T18:41:08.831Z
Learning: In the Atmos configuration import system, errors during config file merging are logged at debug level and the process continues with other imports rather than failing completely, prioritizing resilience over strict correctness.
Applied to files:
pkg/config/load.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:
pkg/config/load_config_args.gocmd/cmd_utils.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
pkg/config/load_config_args.go
📚 Learning: 2025-01-25T03:49:03.951Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: internal/exec/template_utils.go:268-271
Timestamp: 2025-01-25T03:49:03.951Z
Learning: The `ProcessTmplWithDatasourcesGomplate` function in `internal/exec/template_utils.go` is used for documentation generation purposes, where simple environment variable handling is acceptable and thread-safety concerns are not critical.
Applied to files:
cmd/cmd_utils.go
🧬 Code graph analysis (7)
pkg/runner/mock_runner_test.go (1)
pkg/runner/runner.go (1)
AtmosExecParams(44-55)
pkg/schema/command.go (2)
pkg/runner/task.go (1)
Tasks(13-13)pkg/schema/task.go (1)
Tasks(74-74)
cmd/custom_command_integration_test.go (3)
pkg/runner/task.go (2)
Tasks(13-13)Task(9-9)pkg/schema/task.go (2)
Tasks(74-74)Task(32-49)pkg/schema/command.go (1)
Command(5-21)
pkg/config/load.go (2)
pkg/config/const.go (1)
SliceSeparator(154-154)pkg/schema/task.go (1)
TasksDecodeHook(139-159)
pkg/runner/task.go (1)
pkg/schema/task.go (2)
Task(32-49)Tasks(74-74)
pkg/schema/task.go (3)
pkg/schema/command.go (1)
Command(5-21)pkg/schema/schema.go (1)
RetryConfig(784-792)pkg/schema/workflow.go (1)
WorkflowStep(7-15)
cmd/cmd_utils.go (2)
internal/exec/template_utils.go (1)
ProcessTmpl(53-97)pkg/schema/command.go (1)
Command(5-21)
⏰ 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). (1)
- GitHub Check: Summary
🔇 Additional comments (19)
pkg/config/const.go (1)
152-154: LGTM! Clean constant addition.The SliceSeparator constant provides a clear, reusable separator for slice parsing. Good documentation and appropriate placement.
pkg/config/load_config_args.go (1)
48-48: LGTM! Consistent decode hook integration.Adding atmosDecodeHook() to the unmarshal call enables flexible Tasks decoding. This aligns with the same pattern used in pkg/config/load.go (lines 298, 319, 1052) for consistent configuration unmarshaling across the codebase.
cmd/cmd_utils.go (1)
604-604: LGTM! Correct adaptation to Task struct.Updating the template processing to use
step.Commandcorrectly adapts to the new Steps type (schema.Tasks). The template processor expects a string input, so extracting the Command field from the Task struct is the right approach.pkg/runner/task.go (1)
1-13: LGTM! Clean type aliases for package boundary.The Task and Tasks aliases provide a clean interface for the runner package. This is a common Go pattern for establishing clear package boundaries while leveraging shared types from schema.
pkg/config/load.go (1)
14-14: LGTM! Required import for decode hooks.The mapstructure import is necessary for the new atmosDecodeHook() function that composes custom decode behavior.
cmd/custom_command_integration_test.go (2)
18-25: LGTM! Clean test helper.The stepsFromStrings helper provides a clear, reusable way to convert string commands to schema.Tasks for tests. This maintains backward compatibility with test readability while adapting to the new Steps type.
63-63: LGTM! Consistent helper usage.Using stepsFromStrings maintains test clarity while adapting to the schema.Tasks type.
pkg/schema/command.go (2)
3-3: LGTM! Minor punctuation fix.Adding the period follows Go documentation conventions.
13-20: LGTM! Well-documented breaking change with backward compatibility.The Steps field change from []string to Tasks is well-documented and maintains backward compatibility through the TasksDecodeHook. The comments clearly explain the three supported formats (simple, structured, mixed), which helps users understand the enhanced capabilities.
The decode hook (defined in pkg/schema/task.go and wired through atmosDecodeHook) ensures existing YAML with simple string arrays continues to work while enabling new structured step definitions.
pkg/runner/mock_runner_test.go (1)
1-69: LGTM! Generated mock follows guidelines.This is a properly generated mock using go.uber.org/mock/mockgen, as required by the coding guidelines. The generation command (line 6) matches the standard pattern for creating test-only mocks.
As per guidelines: "Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks" - this file adheres to that requirement.
pkg/schema/task.go (9)
1-11: Imports look good!The import organization follows the guidelines with stdlib and 3rd-party packages properly grouped and alphabetically sorted.
13-19: Constants are well-defined.The task type constants are clear and properly documented.
29-49: Task struct is well-designed.The struct has appropriate types, consistent tags, and clear documentation. The use of
time.Durationfor Timeout and pointer for Retry is correct.
51-74: Excellent documentation!The Tasks type documentation clearly illustrates all three supported syntax patterns (simple, structured, mixed), which aligns perfectly with the PR objectives for backward compatibility.
76-107: Verify performance tracking exception for interface implementations.The UnmarshalYAML method is missing the
defer perf.Track()call that the coding guidelines require for all public functions. However, since this implements theyaml.Unmarshalerinterface with a fixed signature, adding performance tracking would require breaking the interface contract.Please confirm whether interface implementations should be exempt from the performance tracking requirement.
The logic for handling both scalar and mapping nodes looks correct, with appropriate error context and default type assignment.
109-134: Verify performance tracking requirement for simple converters.Both
ToWorkflowStepandTaskFromWorkflowStepare public functions missing the requireddefer perf.Track()calls per coding guidelines. Since these are simple field-to-field converters with minimal overhead, please confirm whether performance tracking is needed for such trivial operations.The conversion logic correctly maps all corresponding fields between Task and WorkflowStep, with the comment appropriately noting that Timeout is omitted since WorkflowStep doesn't support it.
136-159: Verify performance tracking for decode hook factory.
TasksDecodeHookis a public function missing the requireddefer perf.Track()call. Since it returns a closure that's executed later by mapstructure, performance tracking at the factory level may not capture the actual decode work. Please confirm the appropriate approach for tracking closure factories.The hook correctly checks types and delegates to the helper for actual decoding.
161-208: Helper functions handle decoding correctly.The internal helpers properly handle:
- String to Task conversion with shell default (line 180)
- Map to Task conversion using mapstructure with duration support (line 195)
- Consistent Type defaulting to TaskTypeShell (lines 180, 204-206)
- Contextual error messages with index information
The use of
StringToTimeDurationHookFunc()ensures timeout strings like "30s" or "5m" are correctly parsed intotime.Duration.
21-27: These sentinel errors are properly defined and used. The guideline "errors MUST be wrapped using static errors" refers to the wrapping mechanism (usingfmt.Errorfwith%w), which is followed here. Your errors are static sentinels wrapped correctly with context, and the pattern of defining package-specific sentinel errors is established throughout the codebase. No action needed.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1901 +/- ##
==========================================
+ Coverage 73.62% 73.66% +0.04%
==========================================
Files 744 746 +2
Lines 67665 67807 +142
==========================================
+ Hits 49815 49950 +135
- Misses 14467 14469 +2
- Partials 3383 3388 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Add additional skip condition for TestExecuteTerraformAffectedComponentInDepOrder that detects when gomonkey partially works but the real function is also called. This happens on macOS where the mock increments callCount but the real ExecuteTerraform is invoked and fails, causing early return before all recursive calls complete. Also fix errorlint violation in pkg/config/load.go by using errors.As instead of type switch for wrapped error handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
fa3dab1 to
f763e2b
Compare
|
Warning Release Documentation RequiredThis PR is labeled
|
Add changelog reference to unified task execution milestone entry, linking it to the blog post at /changelog/unified-task-runner. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
…workflow-compatible-syntax 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…workflow-compatible-syntax
Based on PR merge dates correlated with release tags: v1.202.0 (6 items): - interactive-flag-prompts - automatic-backend-provisioning - global-env-section - workflow-file-auto-discovery - literal-yaml-function - file-scoped-locals v1.201.0 (5 items): - path-based-component-resolution - metadata-inheritance - customizable-list-command-output - custom-command-boolean-flags - version-constraint-validation Also fixes status for: - Unified task execution: shipped → in-progress (PR #1901 not merged) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Add version metadata to shipped roadmap milestones Added an optional `version` field to all 93 shipped milestones in the roadmap, indicating which release each feature was shipped in. Versions are sourced from blog post `release:` fields and git history research. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Display version badge in milestone drawer UI - Add optional version field to Milestone interface - Show version between quarter and status badges in drawer - Style version badge with monospace font and subtle background 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Link version badge to GitHub release notes - Version badge now links to GitHub releases page for that version - Added hover styles for clickable version badge - Opens in new tab with proper security attributes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add version display to FeaturedDrawer component - Add optional version field to FeaturedItem interface - Display version badge with link in featured item drawer - Consistent with MilestoneDrawer version display 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Remove incorrect versions from process-related roadmap items Process/practice improvements don't correspond to specific Atmos releases: - Test coverage improvements (ongoing effort) - Nightly releases (existed before attributed version) - Weekly release cadence (process change) - Claude Code/CodeRabbit integration (tooling) - PR feature releases (CI/CD process) - Fixed Changelog version: v1.198.0 (first blog post date) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Audit and verify all roadmap version metadata - Removed all unverified version fields (87 → 31) - Only keeping versions for items with changelog slugs that have a verified `release:` field in their corresponding blog post - Each version now traceable to its source blog post Verified versions by changelog slug: - introducing-atmos-auth → v1.196.0 - azure-authentication-support → v1.199.0 - flexible-keyring-backends → v1.196.0 - authentication-for-workflows-and-custom-commands → v1.197.0 - atmos-auth-shell → v1.196.0 - describe-commands-identity-flag → v1.197.0 - zero-config-terminal-output → v1.198.0 - introducing-chdir-flag → v1.195.0 - parent-directory-search-and-git-root-discovery → v1.198.0 - atmos-profiles → v1.199.0 - component-aware-stack-completion → v1.198.0 - terminal-themes → v1.198.0 - theme-aware-help → v1.200.0 - helpful-errors → v1.199.0 - provenance-tracking → v1.195.0 - env-function-stack-manifest-support → v1.199.0 - yaml-function-circular-dependency-detection → v1.196.0 - comprehensive-version-management-documentation → v1.199.0 - native-devcontainer-support → v1.201.0 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add verified version metadata from git history research Based on PR merge dates correlated with release tags: v1.202.0 (6 items): - interactive-flag-prompts - automatic-backend-provisioning - global-env-section - workflow-file-auto-discovery - literal-yaml-function - file-scoped-locals v1.201.0 (5 items): - path-based-component-resolution - metadata-inheritance - customizable-list-command-output - custom-command-boolean-flags - version-constraint-validation Also fixes status for: - Unified task execution: shipped → in-progress (PR #1901 not merged) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * address comments * address comments --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com> Co-authored-by: aknysh <andriy.knysh@gmail.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
…workflow-compatible-syntax Resolved conflicts in website/src/data/roadmap.js by: - Keeping version: 'v1.197.0' from main for auth identities milestone - Keeping shipped status, q4-2025 quarter, and changelog from feature branch for unified task execution milestone Merged changes from main include: - ECR authentication integration (PR #1859) - Auth console and manager improvements - Docker config support - Various test improvements and fixes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
website/src/data/roadmap.js (1)
229-230: Update initiative progress to reflect remaining in-progress milestone.With 5 of 6 milestones shipped (line 232 is still 'in-progress'), the progress should be ~83% rather than 100%, and the initiative status should remain 'in-progress'.
As per coding guidelines: progress percentage should be calculated as (shipped milestones / total milestones) * 100.
🔎 Proposed fix
- progress: 100, - status: 'shipped', + progress: 83, + status: 'in-progress',
🧹 Nitpick comments (1)
pkg/config/load_config_args_test.go (1)
117-158: Use schema constant for task type.Line 155 uses the string literal
"shell"instead of theschema.TaskTypeShellconstant. For consistency with other tests (seeatmos_decode_hook_test.goline 83) and to avoid magic strings, use the constant.🔎 Suggested fix
- assert.Equal(t, "shell", atmosConfig.Commands[0].Steps[0].Type) + assert.Equal(t, schema.TaskTypeShell, atmosConfig.Commands[0].Steps[0].Type)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
pkg/config/atmos_decode_hook_test.gopkg/config/load_config_args_test.gopkg/runner/runner_test.gopkg/schema/task_test.gowebsite/blog/2025-12-20-unified-task-runner.mdxwebsite/src/data/roadmap.js
✅ Files skipped from review due to trivial changes (1)
- website/blog/2025-12-20-unified-task-runner.mdx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter)
Never delete existing comments without a very strong reason; preserve helpful comments explaining why/how/what/where, and update comments to match code when refactoring
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
Useflags.NewStandardParser()for command-specific flags; NEVER callviper.BindEnv()orviper.BindPFlag()directly (enforced by Forbidigo)
All errors MUST be wrapped using static errors defined inerrors/errors.go; useerrors.Joinfor combining multiple errors,fmt.Errorfwith%wfor adding string context, error builder for complex errors, anderrors.Is()for error checking. Never use dynamic errors directly
Define interfaces for all major funct...
Files:
pkg/schema/task_test.gopkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_test.gopkg/runner/runner_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: Usecmd.NewTestKit(t)for cmd tests to auto-clean RootCmd state (flags, args)
Test behavior, not implementation; never test stub functions; avoid tautological tests; useerrors.Is()for error checking; remove always-skipped tests
Prefer unit tests with mocks over integration tests; use interfaces and dependency injection for testability; use table-driven tests for comprehensive coverage; target >80% coverage; skip tests gracefully with helpers fromtests/test_preconditions.go
Never manually edit golden snapshot files undertests/test-cases/ortests/testdata/; always use-regenerate-snapshotsflag. Never use pipe redirection when running tests as it breaks TTY detection
Files:
pkg/schema/task_test.gopkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_test.gopkg/runner/runner_test.go
pkg/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/**/*.go: Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions, except trivial getters/setters, command constructors, simple factory functions, and functions that only delegate to another tracked function. Usenilif no atmosConfig param
Avoid adding new functions topkg/utils/; create purpose-built packages for new functionality (e.g.,pkg/store/,pkg/git/,pkg/pro/,pkg/filesystem/)
Files:
pkg/schema/task_test.gopkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_test.gopkg/runner/runner_test.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/src/data/roadmap.js
website/src/data/roadmap.js
📄 CodeRabbit inference engine (CLAUDE.md)
For PRs labeled
minor/major, update roadmap.js: add milestone to relevant initiative withstatus: 'shipped', link to changelog withchangelog: 'your-blog-slug', link to PR withpr: <pr-number>, update initiativeprogresspercentage as(shipped milestones / total milestones) * 100
Files:
website/src/data/roadmap.js
🧠 Learnings (28)
📓 Common learnings
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Stack pipeline: Load atmos.yaml → process imports/inheritance → apply overrides → render templates → generate config. Templates use Go templates + Gomplate with `atmos.Component()`, `!terraform.state`, `!terraform.output`, store integration
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: When adding new CLI command: (1) Create `cmd/[command]/` with CommandProvider interface, (2) Add blank import to `cmd/root.go`, (3) Implement in `internal/exec/mycommand.go`, (4) Add tests and Docusaurus docs in `website/docs/cli/commands/`, (5) Build website: `cd website && npm run build`. See `docs/developing-atmos-commands.md` and `docs/prd/command-registry-pattern.md`
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
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
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.
📚 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/schema/task_test.gopkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_test.gopkg/runner/runner_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/schema/task_test.gopkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_test.gopkg/runner/runner_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/schema/task_test.gopkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_test.gopkg/runner/runner_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; use `errors.Is()` for error checking; remove always-skipped tests
Applied to files:
pkg/schema/task_test.gopkg/runner/runner_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests; use interfaces and dependency injection for testability; use table-driven tests for comprehensive coverage; target >80% coverage; skip tests gracefully with helpers from `tests/test_preconditions.go`
Applied to files:
pkg/schema/task_test.gopkg/runner/runner_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to pkg/datafetcher/schema/**/*.go : Update all schemas in `pkg/datafetcher/schema/` when adding config options
Applied to files:
pkg/schema/task_test.go
📚 Learning: 2025-12-21T04:10:29.030Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration.
Applied to files:
pkg/schema/task_test.gopkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_test.gopkg/runner/runner_test.go
📚 Learning: 2025-12-13T03:21:35.786Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:35.786Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
pkg/config/load_config_args_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to pkg/**/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions, except trivial getters/setters, command constructors, simple factory functions, and functions that only delegate to another tracked function. Use `nil` if no atmosConfig param
Applied to files:
pkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_test.gopkg/runner/runner_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:
pkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_test.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/config/load_config_args_test.gopkg/config/atmos_decode_hook_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:
pkg/config/load_config_args_test.gopkg/runner/runner_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Stack pipeline: Load atmos.yaml → process imports/inheritance → apply overrides → render templates → generate config. Templates use Go templates + Gomplate with `atmos.Component()`, `!terraform.state`, `!terraform.output`, store integration
Applied to files:
pkg/config/load_config_args_test.go
📚 Learning: 2025-12-13T04:37:25.223Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/root.go:0-0
Timestamp: 2025-12-13T04:37:25.223Z
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:
pkg/config/load_config_args_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: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests. This is implemented through custom testing framework extensions and is used consistently throughout the test suite.
Applied to files:
pkg/config/load_config_args_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: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests.
Applied to files:
pkg/config/load_config_args_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Configuration loading precedence: CLI flags → ENV vars → config files → defaults (use Viper). Environment variables require ATMOS_ prefix via `viper.BindEnv("ATMOS_VAR", ...)`
Applied to files:
pkg/config/load_config_args_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: In the atmos codebase, t.Chdir() is a valid method that can be called on *testing.T objects. This functionality is implemented through custom testing framework extensions and is used consistently throughout the test suite for changing working directories during tests.
Applied to files:
pkg/config/load_config_args_test.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
pkg/config/load_config_args_test.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
pkg/config/load_config_args_test.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/config/atmos_decode_hook_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to cmd/**/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions, except trivial getters/setters, command constructors, simple factory functions, and functions that only delegate to another tracked function
Applied to files:
pkg/config/atmos_decode_hook_test.gopkg/runner/runner_test.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
pkg/config/atmos_decode_hook_test.go
📚 Learning: 2024-11-25T17:17:15.703Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Applied to files:
pkg/config/atmos_decode_hook_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Ensure all tests pass, verify code coverage meets targets, run golangci-lint and fix any issues, and update documentation before submitting pull requests
Applied to files:
pkg/runner/runner_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:
pkg/runner/runner_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to website/src/data/roadmap.js : For PRs labeled `minor`/`major`, update roadmap.js: add milestone to relevant initiative with `status: 'shipped'`, link to changelog with `changelog: 'your-blog-slug'`, link to PR with `pr: <pr-number>`, update initiative `progress` percentage as `(shipped milestones / total milestones) * 100`
Applied to files:
website/src/data/roadmap.js
🧬 Code graph analysis (4)
pkg/schema/task_test.go (5)
pkg/schema/task.go (6)
TaskTypeShell(16-16)TaskTypeAtmos(18-18)ErrTaskInvalidFormat(24-24)ErrTaskUnexpectedNodeKind(26-26)TaskFromWorkflowStep(123-134)TasksDecodeHook(139-159)cmd/cmd_utils.go (1)
Contains(1303-1310)pkg/schema/schema.go (1)
RetryConfig(834-842)pkg/schema/workflow.go (1)
WorkflowStep(7-15)pkg/ui/heatmap/heatmap.go (1)
Steps(62-62)
pkg/config/load_config_args_test.go (1)
pkg/schema/schema.go (3)
ConfigAndStacksInfo(716-813)AtmosConfiguration(54-100)Logs(534-537)
pkg/config/atmos_decode_hook_test.go (1)
pkg/schema/task.go (2)
TaskTypeShell(16-16)TaskTypeAtmos(18-18)
pkg/runner/runner_test.go (2)
pkg/schema/task.go (2)
Task(32-49)Tasks(74-74)pkg/runner/runner.go (5)
Options(58-69)Run(74-107)AtmosExecParams(44-55)ErrUnknownTaskType(18-18)RunAll(159-168)
⏰ 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 (15)
website/src/data/roadmap.js (1)
237-237: Milestone update looks good.The milestone correctly reflects this PR's completion: status updated to 'shipped', quarter moved to 'q4-2025', changelog and PR links added. Well done.
pkg/config/atmos_decode_hook_test.go (1)
1-120: Strong test coverage for decode hooks.The tests comprehensively cover the atmosDecodeHook functionality including duration conversion, slice parsing, and Tasks decoding with both simple and structured syntax. Proper use of schema constants and clear test structure.
pkg/config/load_config_args_test.go (1)
1-235: Comprehensive test coverage for config loading from CLI args.The test suite thoroughly exercises config loading from files and directories, validation helpers, and path utilities. Good coverage of both success and error paths. Tests are focused and readable.
pkg/schema/task_test.go (5)
1-11: LGTM - imports well organized.Imports correctly follow the three-group convention with stdlib first, then third-party. No atmos internal imports needed since this is testing the same package.
13-198: Solid YAML unmarshal test coverage.Tests cover the key scenarios: simple strings, structured maps, mixed syntax, defaults, retry config, error cases. Good use of
require.NoErrorfor critical assertions andassert.ErrorIsfor sentinel error checks.
199-248: Good round-trip conversion tests.Both directions tested. The comment on line 222 noting Timeout isn't in WorkflowStep is helpful context.
258-279: Good mapstructure decode hook test pattern.Clear setup with DecoderConfig and WeaklyTypedInput. This validates the hook works correctly in the mapstructure pipeline.
492-500: Empty commands are intentionally allowed by design.The test documents that empty maps decode with empty commands defaulting to shell type. While the Command field lacks an
omitemptytag (unlike other optional fields), no validation enforces non-empty commands throughout the decode or execution pipeline. This appears intentional to support flexible task definitions, though adding early validation with clear error messages could improve user experience over failing silently at runtime.pkg/runner/runner_test.go (7)
1-14: Imports well structured.Correct three-group organization. gomock and testify usage aligns with project guidelines.
16-38: Solid shell task test.Good mock setup with explicit parameter verification. The pattern of ctrl.Finish() in defer is consistent across all tests.
207-233: Well-designed timeout test.Verifying the context deadline inside DoAndReturn is a smart approach. The 50ms tolerance in
WithinDurationaccounts for execution time variance.
235-296: Good sequential execution tests.
gomock.InOrdercorrectly enforces execution order. Early termination behavior verified with partial expectations.
298-308: Correct stack arg insertion tests.The separator handling case (line 304-308) is important - ensures
-sflag appears before--so it's interpreted by atmos, not passed through.
369-396: Nice fallback behavior test.Testing the shell.Fields error fallback to strings.Fields is defensive. The comment explains the malformed quoting scenario well.
504-513: Clear taskName helper tests.Tests confirm 1-based step naming (
step1at index 0,step5at index 4).
|
These changes were released in v1.204.0-rc.0. |
what
pkg/runnerpackage with unified Task type and CommandRunner interfacewhy
Provides a shared execution layer for both custom commands and workflows, enabling code reuse and consistent task execution patterns across Atmos. Fixes DEV-3005.
references
Closes #DEV-3005
Summary by CodeRabbit
New Features
Breaking Change
Tests
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.