Insert workflow-level stack flag before -- in workflow args
#1411
Conversation
📝 WalkthroughWalkthroughAdjusts atmos command argument assembly in ExecuteWorkflow to insert Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ExecuteWorkflow
participant ArgBuilder
participant Atmos
User->>CLI: run workflow
CLI->>ExecuteWorkflow: ExecuteWorkflow(opts)
ExecuteWorkflow->>ArgBuilder: build args
ArgBuilder->>ArgBuilder: find "--" via slices.Index
alt "--" found
ArgBuilder->>ArgBuilder: insert "-s", stack before "--"
else
ArgBuilder->>ArgBuilder: append "-s", stack at end
end
ExecuteWorkflow->>Atmos: invoke with args
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/exec/workflow_utils.go (1)
141-151: Use slices.Insert for clarity; optionally de-dupe existing stack flags.
- Readability: slices.Insert makes the intent obvious.
- Optional: if the original command already includes a stack flag before “--”, consider removing it so the injected one is the single source of truth.
Apply:
- if idx := slices.Index(args, "--"); idx != -1 { - // Insert before the "--" - // Take everything up to idx, then add "-s", finalStack, then tack on the rest - args = append(args[:idx], append([]string{"-s", finalStack}, args[idx:]...)...) - } else { - // just append at the end - args = append(args, "-s", finalStack) - } + if idx := slices.Index(args, "--"); idx != -1 { + // Insert "-s <stack>" immediately before "--" + args = slices.Insert(args, idx, "-s", finalStack) + } else { + args = append(args, "-s", finalStack) + }Notes:
- If you want to avoid duplicate stack flags, prune any existing “-s/--stack” before the sentinel prior to insertion.
- Also consider aligning the user-facing “Executing command” message to display the mutated args so it reflects the actual invocation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/exec/workflow_utils.go(2 hunks)
🧰 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
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Files:
internal/exec/workflow_utils.go
🧠 Learnings (6)
📓 Common learnings
Learnt from: aknysh
PR: cloudposse/atmos#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.
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
internal/exec/workflow_utils.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
PR: cloudposse/atmos#764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
internal/exec/workflow_utils.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
internal/exec/workflow_utils.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
PR: cloudposse/atmos#1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
internal/exec/workflow_utils.go
📚 Learning: 2025-02-21T20:56:05.539Z
Learnt from: samtholiya
PR: cloudposse/atmos#1077
File: pkg/downloader/custom_github_detector.go:0-0
Timestamp: 2025-02-21T20:56:05.539Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log` according to the project's import alias configuration.
Applied to files:
internal/exec/workflow_utils.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (1)
internal/exec/workflow_utils.go (1)
10-10: Go stdlib “slices” is supported
The repo’s go.mod specifies Go 1.24.5, which includes the “slices” package (added in Go 1.21). No changes required.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
internal/exec/workflow_utils.go (3)
141-149: Stack flag insertion before--works; consider two polish items.The insertion logic is correct and meets the PR goal. Two optional refinements:
- Consider stripping any existing pre-
--occurrences of-s/--stackto avoid duplicate flags.- Add a brief comment noting that
--terminates Atmos flag parsing (future readers will thank you).No blocker—just quality-of-life improvements.
154-154: Show the actual executed args in the TUI message.This currently prints the original
commandstring and not the arg list after inserting-s. Display the joinedargsso users see the real invocation.Apply:
- u.PrintfMessageToTUI("Executing command: `atmos %s`\n", command) + u.PrintfMessageToTUI("Executing command: `atmos %s`\n", strings.Join(args, " "))
186-193: Make the failure message reflect the actual invocation (insert-sbefore--).When building
failedCmd, appending-sto the end will place it after--if present, which misrepresents how the command was executed. Construct the message using the same insertion logic used forargs.Apply:
- failedCmd = config.AtmosCommand + " " + command - // Add stack parameter to failed command if a stack was used - if finalStack != "" { - failedCmd = fmt.Sprintf("%s -s %s", failedCmd, finalStack) - } + // Build a faithful representation of the executed command (stack flag before any "--") + argsForMsg := strings.Fields(command) + if finalStack != "" { + if i := slices.Index(argsForMsg, "--"); i != -1 { + argsForMsg = append(argsForMsg[:i], append([]string{"-s", finalStack}, argsForMsg[i:]...)...) + } else { + argsForMsg = append(argsForMsg, "-s", finalStack) + } + } + failedCmd = config.AtmosCommand + " " + strings.Join(argsForMsg, " ")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/exec/workflow_utils.go(2 hunks)
🧰 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
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Files:
internal/exec/workflow_utils.go
🧠 Learnings (9)
📓 Common learnings
Learnt from: aknysh
PR: cloudposse/atmos#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.
Learnt from: Cerebrovinny
PR: cloudposse/atmos#764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
internal/exec/workflow_utils.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
PR: cloudposse/atmos#764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
internal/exec/workflow_utils.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
internal/exec/workflow_utils.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
PR: cloudposse/atmos#1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
internal/exec/workflow_utils.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
PR: cloudposse/atmos#740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
internal/exec/workflow_utils.go
📚 Learning: 2024-10-30T13:25:45.965Z
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:405-412
Timestamp: 2024-10-30T13:25:45.965Z
Learning: In `internal/exec/terraform_clean.go`, when appending `stackFolders` to `folders` in the `handleCleanSubCommand` function, it's unnecessary to check if `stackFolders` is nil before appending, because in Go, appending a nil slice is safe and does not cause a panic.
Applied to files:
internal/exec/workflow_utils.go
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
PR: cloudposse/atmos#825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
internal/exec/workflow_utils.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
PR: cloudposse/atmos#795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
internal/exec/workflow_utils.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (1)
internal/exec/workflow_utils.go (1)
8-8: stdlib “slices” is supported (Go 1.24.5)
go.mod specifiesgo 1.24.5(≥ 1.21), so the standard-libraryslicesimport is valid. No changes required.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1411 +/- ##
==========================================
+ Coverage 55.29% 55.33% +0.03%
==========================================
Files 272 272
Lines 28481 28487 +6
==========================================
+ Hits 15748 15762 +14
+ Misses 10957 10946 -11
- Partials 1776 1779 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
These changes were released in v1.188.0-test.0. |
This PR updates the workflow utils so that when a workflow-level
stackis provided, the generated-s <stack>flag is placed before any--argument in the command args (instead of always at the end).The
--marker indicates the end of Atmos CLI arguments; anything after it is passed directly to the underlying tool or script. Appending-s <stack>after--caused the stack flag to be ignored in such cases.Changes
workflow_utils.go, modified the logic around appending-s <stack>:--exists inargs, insert the stack flag immediately before it.--is present, behavior remains the same (flag is appended at the end).Example
Before
After
Summary by CodeRabbit