fix: disable pager mouse interactions and add global --pager flag #1430
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughCentralizes pager handling via a new global --pager flag and Terminal methods. Removes per-command pager wiring, updates config/env parsing, defaults, and schema (adds Color, methods). Adjusts pager behavior (disabled by default). Updates help, tests, and docs. Increases Windows retry delays. Changes default log level to Warning. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (root)
participant Cfg as cfg.InitCliConfig
participant Env as Env/Flags
participant Schema as Terminal (schema)
participant Pager as Pager
User->>CLI: Run atmos [cmd] [--pager[=VALUE]] [--no-color]
CLI->>Cfg: Initialize config
Cfg->>Env: Read flags/env (pager, color/no-color)
Env-->>Cfg: Parsed settings
Cfg->>Schema: Populate Terminal{Pager, Color, NoColor}
Schema-->>Cfg: Methods IsPagerEnabled()/IsColorEnabled()
CLI->>Schema: Decide pager/color for output
alt pager enabled
CLI->>Pager: NewWithAtmosConfig(true)
Pager-->>CLI: PageCreator
CLI->>Pager: Run(content)
else pager disabled
CLI-->>User: Print to stdout
end
sequenceDiagram
autonumber
actor User
participant CLI as CLI (--help)
participant Schema as Terminal
participant Pager as Pager
User->>CLI: atmos [cmd] --help [--pager[=VALUE]]
CLI->>Schema: IsPagerEnabled()
alt enabled
CLI->>Pager: NewWithAtmosConfig(true)
Pager->>Pager: Init (alt-screen)
Pager-->>CLI: Ready
CLI->>Pager: Run(rendered help)
else disabled
CLI-->>User: Print help to stdout
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
pkg/config/load.go (1)
138-141: Defaults conflict with PR: pager should default to false; also set explicit default for color=true.Currently v.SetDefault("settings.terminal.pager", true) contradicts the new default (disabled). Also, without an explicit default for settings.terminal.color, a zero-value bool may unintentionally disable color depending on IsColorEnabled logic.
Apply:
v.SetDefault("settings.terminal.no_color", false) -v.SetDefault("settings.terminal.pager", true) +// Pager is disabled by default (can be enabled via config/env/flag). +v.SetDefault("settings.terminal.pager", false) +// Make color explicitly default-on; NO_COLOR still overrides via IsColorEnabled(). +v.SetDefault("settings.terminal.color", true) v.SetDefault("docs.generate.readme.output", "./README.md")Please add/adjust tests to assert:
- Default pager is false when no config/env/flags are provided.
- NO_COLOR overrides COLOR/ATMOS_COLOR and settings.terminal.color.
cmd/root.go (2)
101-111: Honor new color semantics (use IsColorEnabled).This keeps behavior consistent when users set
color: false(even ifno_coloris not set).- if atmosConfig.Settings.Terminal.NoColor { + if !atmosConfig.Settings.Terminal.IsColorEnabled() { stylesDefault := log.DefaultStyles() // Clear colors for levels styles := &log.Styles{} styles.Levels = make(map[log.Level]lipgloss.Style) for k := range stylesDefault.Levels { styles.Levels[k] = stylesDefault.Levels[k].UnsetForeground().Bold(false) } log.SetStyles(styles) }
289-293: Shadowing imported package namepager.Short var
pager := pager.New...shadows the imported package and will failimportshadowlinters.- pager := pager.NewWithAtmosConfig(pagerEnabled) - if err := pager.Run("Atmos CLI Help", buf.String()); err != nil { + pg := pager.NewWithAtmosConfig(pagerEnabled) + if err := pg.Run("Atmos CLI Help", buf.String()); err != nil { log.Error("Failed to run pager", "error", err) utils.OsExit(1) }
♻️ Duplicate comments (1)
cmd/root.go (1)
273-287: Normalize and switch on --pager value; simplifies and fixes case/whitespace.Also addresses the linter’s if-else chain warning.
- // Check if pager should be enabled based on flag, env var, or config - pagerEnabled := atmosConfig.Settings.Terminal.IsPagerEnabled() - - // Check if --pager flag was explicitly set - if pagerFlag, err := command.Flags().GetString("pager"); err == nil && pagerFlag != "" { - // Handle --pager flag values - if pagerFlag == "true" || pagerFlag == "on" || pagerFlag == "yes" || pagerFlag == "1" { - pagerEnabled = true - } else if pagerFlag == "false" || pagerFlag == "off" || pagerFlag == "no" || pagerFlag == "0" { - pagerEnabled = false - } else { - // Assume it's a pager command like "less" or "more" - pagerEnabled = true - } - } + // Start with config/env; override with --pager if explicitly set + pagerEnabled := atmosConfig.Settings.Terminal.IsPagerEnabled() + if pagerFlagRaw, err := command.Flags().GetString("pager"); err == nil && pagerFlagRaw != "" { + pf := strings.ToLower(strings.TrimSpace(pagerFlagRaw)) + switch pf { + case "true", "on", "yes", "1": + pagerEnabled = true + case "false", "off", "no", "0": + pagerEnabled = false + default: + // Treat any non-boolean value as a pager command (e.g. "less", "more", "less -SR") + pagerEnabled = true + } + }
🧹 Nitpick comments (14)
website/docs/cli/global-flags.mdx (2)
312-317: Clarify PAGER=cat semantics.“Disables paging” is misleading; using cat bypasses interactive paging by writing directly to stdout.
Apply:
-export PAGER=cat # Disables paging in Atmos and other tools that respect PAGER +export PAGER=cat # Bypasses interactive paging (outputs directly to stdout)
356-361: Missing punctuation.End the last bullet with a period for consistency.
-- **Better defaults**: Most modern terminals have built-in scrollback +- **Better defaults**: Most modern terminals have built-in scrollback.pkg/config/load.go (1)
231-249: Optional: unify env access via Viper.readEnvAmosConfigPath uses os.Getenv; consider viper.AutomaticEnv + v.GetString for consistency with the rest of the config pipeline.
-func readEnvAmosConfigPath(v *viper.Viper) error { - atmosPath := os.Getenv("ATMOS_CLI_CONFIG_PATH") +func readEnvAmosConfigPath(v *viper.Viper) error { + _ = v.BindEnv("ATMOS_CLI_CONFIG_PATH") + atmosPath := v.GetString("ATMOS_CLI_CONFIG_PATH")pkg/pager/pager.go (2)
55-57: Wrap Bubble Tea error with context.
Add context to aid debugging without losing the original error.- ).Run(); err != nil { - return err + ).Run(); err != nil { + return fmt.Errorf("pager: bubbletea program.Run failed: %w", err) }
11-14: Add GoDoc for exported API.
Exported type and constructors lack comments; add concise docs for godoc and linters.//go:generate mockgen -source=$GOFILE -destination=mock_$GOFILE -package=$GOPACKAGE type PageCreator interface { Run(title, content string) error } type pageCreator struct {+// NewWithAtmosConfig returns a PageCreator honoring the provided pager enablement. func NewWithAtmosConfig(enablePager bool) PageCreator {-func New() PageCreator { +// New returns a PageCreator with paging disabled by default. +func New() PageCreator {Also applies to: 23-36
pkg/schema/schema_test.go (1)
70-86: Broaden pager tests for negative uppercase cases.
Add OFF/FALSE uppercase to mirror ON/TRUE coverage.{ - {"Capitalized 'ON' should enable pager", "ON", true}, - {"Capitalized 'TRUE' should enable pager", "TRUE", true}, + {"Capitalized 'ON' should enable pager", "ON", true}, + {"Capitalized 'TRUE' should enable pager", "TRUE", true}, + {"Capitalized 'OFF' should disable pager", "OFF", false}, + {"Capitalized 'FALSE' should disable pager", "FALSE", false}, }website/docs/cli/configuration/configuration.mdx (2)
122-127: Document precedence explicitly in the defaults snippet.Suggest adding a one-line note so users see precedence at a glance.
max_width: 120 # Maximum width for terminal output pager: false # Pager disabled by default (set to true or pager command to enable) + # Precedence: flags > environment variables > config file > defaults
276-287: Clarify color env vars and precedence; note NO_COLOR wins.Make precedence and variables unambiguous to prevent surprises.
- <dd>Enable or disable colored output (default: `true`). Can be overridden with `--no-color` flag or `NO_COLOR`/`ATMOS_NO_COLOR` environment variables.</dd> + <dd>Enable or disable colored output (default: `true`). Precedence: flags > env > config. Supports `ATMOS_COLOR`/`COLOR` to force enable/disable; `NO_COLOR`/`ATMOS_NO_COLOR` always disable and take precedence over `COLOR`.</dd> @@ - **Environment Variables Still Supported**: The `NO_COLOR` and `ATMOS_NO_COLOR` environment variables remain fully supported for portability across different environments and CI/CD systems. + **Environment Variables Still Supported**: `NO_COLOR` and `ATMOS_NO_COLOR` remain fully supported and take precedence over `COLOR`/`ATMOS_COLOR`. `ATMOS_COLOR` and `COLOR` can be used to enable or disable color when `NO_COLOR` is not set.pkg/schema/schema.go (1)
223-231: Fix comment to match behavior.The function cannot infer an "unset" bool; defaults come from config layer.
- // Use Color setting (defaults to true if not explicitly set) + // Use Color setting; deprecated NoColor overridespkg/pager/pager_test.go (2)
97-100: Avoid brittle count assertion on ProgramOptions.Assert presence (>=1) rather than an exact count.
- assert.Len(t, capturedOpts, 1, "Should have 1 tea program option (WithAltScreen)") + assert.GreaterOrEqual(t, len(capturedOpts), 1, "Should include WithAltScreen option")
228-230: Mirror the non-brittle option check here too.- assert.Len(t, capturedOpts, 1, "Should have 1 tea program option (WithAltScreen)") + assert.GreaterOrEqual(t, len(capturedOpts), 1, "Should include WithAltScreen option")cmd/root.go (1)
212-215: Tighten flag help text: default and precedence.Clarify behavior and examples.
- RootCmd.PersistentFlags().String("pager", "", "Enable pager for output (--pager or --pager=true to enable, --pager=false to disable, --pager=less to use specific pager)") + RootCmd.PersistentFlags().String("pager", "", "Control paging of help/long output (default: disabled). Use --pager or --pager=true to enable, --pager=false to disable, or --pager='less -SR' to use a specific pager. Flag overrides config/env.")pkg/config/config_test.go (2)
532-611: Expand parseFlags coverage for --pager synonyms and bare flagsDocs mention accepting true/false/command; they also say “on”. Add cases to ensure we parse "--pager=on" and the bare "--pager" form consistently.
Apply:
@@ func TestParseFlagsForPager(t *testing.T) { { name: "pager flag with true", args: []string{"atmos", "describe", "config", "--pager=true"}, expectedPager: "true", expectedNoColor: false, }, + { + name: "pager flag with on", + args: []string{"atmos", "describe", "config", "--pager=on"}, + expectedPager: "on", + expectedNoColor: false, + }, + { + name: "bare pager flag enables pager", + args: []string{"atmos", "describe", "config", "--pager"}, + expectedPager: "true", + expectedNoColor: false, + },
613-667: Cover boolean variants in setLogConfig (pager true/false)Add tests to ensure CLI-only toggles work without env influence.
Apply:
@@ func TestSetLogConfigWithPager(t *testing.T) { tests := []struct { name string args []string expectedPager string expectedNoColor bool expectedColor bool }{ { name: "pager from flag", args: []string{"atmos", "--pager=less"}, expectedPager: "less", expectedNoColor: false, }, + { + name: "pager true via bare flag", + args: []string{"atmos", "--pager"}, + expectedPager: "true", + expectedNoColor: false, + }, + { + name: "pager false via flag", + args: []string{"atmos", "--pager=false"}, + expectedPager: "false", + expectedNoColor: false, + }, { name: "no-color flag sets both NoColor and Color", args: []string{"atmos", "--no-color"}, expectedPager: "", expectedNoColor: true, expectedColor: false, },
📜 Review details
Configuration used: Path: .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 (18)
atmos.yaml(1 hunks)cmd/describe.go(0 hunks)cmd/describe_affected.go(1 hunks)cmd/describe_config.go(1 hunks)cmd/describe_dependents.go(1 hunks)cmd/describe_stacks.go(1 hunks)cmd/describe_workflows.go(1 hunks)cmd/root.go(2 hunks)pkg/config/config.go(1 hunks)pkg/config/config_test.go(1 hunks)pkg/config/load.go(1 hunks)pkg/pager/pager.go(2 hunks)pkg/pager/pager_test.go(4 hunks)pkg/schema/schema.go(1 hunks)pkg/schema/schema_test.go(2 hunks)website/docs/cli/configuration/configuration.mdx(3 hunks)website/docs/cli/configuration/terminal.mdx(3 hunks)website/docs/cli/global-flags.mdx(1 hunks)
💤 Files with no reviewable changes (1)
- cmd/describe.go
🧰 Additional context used
📓 Path-based instructions (4)
cmd/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/*.go: Implement each Cobra command in a separate file under the cmd/ directory
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in command help
Provide meaningful feedback to users in command implementation
Include progress indicators for long-running operations
Provide clear error messages to users
Include troubleshooting hints when appropriate
Log detailed errors for debugging
Files:
cmd/describe_workflows.gocmd/describe_dependents.gocmd/describe_affected.gocmd/describe_stacks.gocmd/root.gocmd/describe_config.go
**/*.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:
cmd/describe_workflows.gocmd/describe_dependents.gopkg/config/load.gocmd/describe_affected.gopkg/config/config.gopkg/pager/pager.gopkg/schema/schema.gocmd/describe_stacks.gocmd/root.gopkg/schema/schema_test.gocmd/describe_config.gopkg/config/config_test.gopkg/pager/pager_test.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in the website/ directory when adding new features
Follow the website's documentation structure and style
Keep website code in the website/ directory
Follow the existing website architecture and style
Document new features on the website
Include examples and use cases in website documentation
Files:
website/docs/cli/global-flags.mdxwebsite/docs/cli/configuration/terminal.mdxwebsite/docs/cli/configuration/configuration.mdx
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
Files:
pkg/schema/schema_test.gopkg/config/config_test.gopkg/pager/pager_test.go
🧠 Learnings (19)
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
cmd/describe_workflows.gocmd/describe_dependents.gocmd/describe_affected.gopkg/config/config.gocmd/describe_stacks.gocmd/root.gocmd/describe_config.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
PR: cloudposse/atmos#1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
cmd/describe_workflows.gocmd/describe_dependents.gocmd/describe_stacks.gocmd/describe_config.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
PR: cloudposse/atmos#1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
cmd/describe_dependents.gocmd/describe_stacks.gocmd/describe_config.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:
cmd/describe_dependents.gocmd/describe_config.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
pkg/config/load.gopkg/config/config.gopkg/schema/schema.go
📚 Learning: 2025-04-10T20:48:22.687Z
Learnt from: samtholiya
PR: cloudposse/atmos#1147
File: pkg/config/load.go:0-0
Timestamp: 2025-04-10T20:48:22.687Z
Learning: In the `bindEnv` function in `pkg/config/load.go`, panic is used deliberately instead of returning errors because errors from `BindEnv` would only occur due to developer mistakes. Using panic helps with early detection of these developer errors during initialization.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
PR: cloudposse/atmos#1202
File: pkg/utils/yaml_func_exec.go:104-104
Timestamp: 2025-04-23T15:02:50.246Z
Learning: In the Atmos codebase, direct calls to `os.Getenv` should be avoided. Instead, use `viper.BindEnv` for environment variable access. This provides a consistent approach to configuration management across the codebase.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
PR: cloudposse/atmos#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/config/load.gopkg/config/config_test.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
PR: cloudposse/atmos#1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
cmd/describe_affected.gocmd/describe_stacks.go
📚 Learning: 2024-12-12T17:13:53.409Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#848
File: pkg/utils/doc_utils.go:19-22
Timestamp: 2024-12-12T17:13:53.409Z
Learning: In `pkg/utils/doc_utils.go`, the `DisplayDocs` function uses the `PAGER` environment variable, which is intentionally user-configurable to allow users to specify custom pager commands that fit their workflow; adding validation to restrict it is not desired.
Applied to files:
cmd/describe_affected.gopkg/pager/pager.gocmd/root.gocmd/describe_config.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:
cmd/describe_stacks.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Provide comprehensive help text for all commands and flags
Applied to files:
cmd/root.go
📚 Learning: 2024-12-11T18:46:02.483Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/terraform.go:39-39
Timestamp: 2024-12-11T18:46:02.483Z
Learning: `cliConfig` is initialized in `cmd/root.go` and can be used across the `cmd` package.
Applied to files:
cmd/root.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/root.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
pkg/schema/schema_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios
Applied to files:
pkg/schema/schema_test.gopkg/pager/pager_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Follow the precedence order: flags > environment variables > config file > defaults
Applied to files:
pkg/config/config_test.go
📚 Learning: 2025-02-20T13:57:36.326Z
Learnt from: osterman
PR: cloudposse/atmos#1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands supports -1 for unlimited width, which bypasses terminal width restrictions and may require horizontal scrolling. The default is 10 columns if not specified.
Applied to files:
website/docs/cli/configuration/terminal.mdx
📚 Learning: 2025-01-09T19:53:29.847Z
Learnt from: Listener430
PR: cloudposse/atmos#912
File: pkg/config/config.go:91-92
Timestamp: 2025-01-09T19:53:29.847Z
Learning: In the Atmos project, the `core.inject_github_token` configuration is required to be enabled (`true`) by default to support authenticated GitHub requests and help bypass rate limits.
Applied to files:
website/docs/cli/configuration/configuration.mdx
🧬 Code graph analysis (5)
pkg/config/config.go (1)
pkg/schema/schema.go (2)
Settings(851-855)Terminal(203-211)
cmd/root.go (2)
pkg/schema/schema.go (2)
Settings(851-855)Terminal(203-211)pkg/pager/pager.go (1)
NewWithAtmosConfig(23-27)
pkg/schema/schema_test.go (1)
pkg/schema/schema.go (1)
Terminal(203-211)
pkg/config/config_test.go (1)
pkg/schema/schema.go (4)
AtmosConfiguration(25-60)Settings(851-855)AtmosSettings(243-263)Terminal(203-211)
pkg/pager/pager_test.go (1)
pkg/pager/pager.go (1)
NewWithAtmosConfig(23-27)
🪛 LanguageTool
website/docs/cli/global-flags.mdx
[grammar] ~7-~7: There might be a mistake here.
Context: ...Intro from '@site/src/components/Intro'; import Note from '@site/src/components/N...
(QB_NEW_EN)
[grammar] ~183-~183: There might be a mistake here.
Context: ...1. Command-line flags (highest priority) 2. Environment variables 3. Configuration f...
(QB_NEW_EN)
[grammar] ~184-~184: There might be a mistake here.
Context: ...ghest priority) 2. Environment variables 3. Configuration file (atmos.yaml) 4. Def...
(QB_NEW_EN)
[grammar] ~185-~185: There might be a mistake here.
Context: ...les 3. Configuration file (atmos.yaml) 4. Default values (lowest priority) ### Co...
(QB_NEW_EN)
[grammar] ~360-~360: Please add a punctuation mark at the end of paragraph.
Context: ...**: Most modern terminals have built-in scrollback ## See Also - [CLI Configuration](/cl...
(PUNCTUATION_PARAGRAPH_END)
🪛 GitHub Check: golangci-lint
cmd/root.go
[failure] 279-279:
ifElseChain: rewrite if-else to switch statement
⏰ 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). (3)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Summary
🔇 Additional comments (17)
atmos.yaml (1)
352-357: Pager default switched off — good.This aligns atmos.yaml with the PR’s breaking change and docs.
cmd/describe_affected.go (1)
73-76: Global --pager is already applied in ParseDescribeAffectedCliArgs
ParseDescribeAffectedCliArgs invokes cfg.InitCliConfig(info, true), which processes the global --pager flag into atmosConfig.Settings.Terminal.Pager, and then SetDescribeAffectedFlagValueInCliArgs maps “pager” onto props.CLIConfig.Settings.Terminal.Pager. No changes needed here.website/docs/cli/global-flags.mdx (1)
322-329: Ensure auto-disable implemented in CLI code
Doc states that paging and color are disabled when output is piped, but no TTY-detection (e.g.isatty/terminal.IsTerminal) was found in the code. Confirm the CLI checks stdout for a real TTY and disables pager/color on pipes, or update the docs to match actual behavior.cmd/describe_workflows.go (1)
55-57: LGTM: pager handling centralized.This command path initializes via cfg.InitCliConfig and passes atmosConfig through; removing local pager handling is correct.
pkg/pager/pager.go (2)
31-31: Default pager now disabled — verify precedence and UX copy.
Confirm flags > env vars > config > defaults behavior is preserved end-to-end and docs/examples reflect the new default.
54-55: Mouse motion tracking removed: selection fix looks good.
Dropping tea.WithMouseCellMotion avoids capturing drag events. Nice.cmd/describe_dependents.go (1)
57-57: Pager handling centralized: LGTM.
Removing per-command overrides reduces duplication and honors global init.cmd/describe_stacks.go (1)
73-73: Pager handling centralized: LGTM.
Consistent with other describe commands and root-level flag.pkg/schema/schema_test.go (1)
40-62: Color precedence tests read well.
Covers all key combinations; clear expectation that NoColor wins.pkg/config/config.go (1)
87-90: Global --pager propagation: OK.
Simple assignment keeps precedence consistent with other flags.pkg/schema/schema.go (1)
208-211: Public color fields: OK, but ensure deprecation path is surfaced in docs/tests.Fields and tags look fine; rely on IsColorEnabled() where possible.
Please ensure tests cover both Color=false and NoColor=true interactions (including env overrides).
pkg/pager/pager_test.go (2)
173-190: Constructor coverage: LGTM.Good coverage of enabled/disabled toggles.
240-246: Explicit “pager disabled” case: nice.Covers the new default well.
website/docs/cli/configuration/terminal.mdx (4)
30-33: Defaults align with PR (pager off, color on)Matches stated behavior. Good.
37-45: Clear deprecation messagingGood split between config deprecation and env portability.
115-115: Syntax-highlighting pager independence is clearGood clarification.
61-91: Reflect ‘on’ support & document precedence
- Update pager docs in website/docs/cli/configuration/terminal.mdx to
“- true or on: Enable pager with system default” (tests confirmonis accepted).- Append a tip under the
<dl>outlining Atmos’s setting resolution order:
- CLI flags
- Environment variables
- Config file
- Defaults
Verify code respects this precedence.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
website/docs/cli/global-flags.mdx (2)
254-268: Add arg-passing examples for pager.Show common real-world forms with quotes.
Apply:
# Enable pager (multiple ways) atmos describe config --pager # Enable with default pager atmos describe config --pager=true # Explicitly enable - atmos describe config --pager=less # Use specific pager + atmos describe config --pager=less # Use specific pager + atmos describe config --pager="less -RFX" # Pager with arguments ATMOS_PAGER=true atmos describe config # Via environment variable + PAGER="less -RFX" atmos describe config # Standard env var with arguments
312-315: Reword: PAGER=cat doesn’t “disable” paging; it pass-throughs.Avoid implying special handling. It effectively bypasses paging.
Apply:
-export PAGER=cat # Disables paging in Atmos and other tools that respect PAGER +export PAGER=cat # Effectively bypasses paging by streaming output
📜 Review details
Configuration used: Path: .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 (2)
cmd/root.go(2 hunks)website/docs/cli/global-flags.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/root.go
🧰 Additional context used
📓 Path-based instructions (1)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in the website/ directory when adding new features
Follow the website's documentation structure and style
Keep website code in the website/ directory
Follow the existing website architecture and style
Document new features on the website
Include examples and use cases in website documentation
Files:
website/docs/cli/global-flags.mdx
🧠 Learnings (1)
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Follow the precedence order: flags > environment variables > config file > defaults
Applied to files:
website/docs/cli/global-flags.mdx
🪛 LanguageTool
website/docs/cli/global-flags.mdx
[grammar] ~7-~7: There might be a mistake here.
Context: ...Intro from '@site/src/components/Intro'; import Note from '@site/src/components/N...
(QB_NEW_EN)
[grammar] ~183-~183: There might be a mistake here.
Context: ...1. Command-line flags (highest priority) 2. Environment variables 3. Configuration f...
(QB_NEW_EN)
[grammar] ~184-~184: There might be a mistake here.
Context: ...ghest priority) 2. Environment variables 3. Configuration file (atmos.yaml) 4. Def...
(QB_NEW_EN)
[grammar] ~185-~185: There might be a mistake here.
Context: ...les 3. Configuration file (atmos.yaml) 4. Default values (lowest priority) ### Co...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (5)
website/docs/cli/global-flags.mdx (5)
181-187: Precedence order is correctly documented.Matches the established rule: flags > env vars > config > defaults.
322-329: Verify auto-disable behavior when piped; tighten wording.Confirm the implementation disables the pager when stdout is non-TTY, and be explicit about color behavior.
Apply:
-<Note> -When output is piped to another command, Atmos automatically disables color output and pager to ensure compatibility: +<Note> +When output is piped (stdout is not a TTY), Atmos automatically disables the pager. Color follows the usual precedence (flags/env/config); use `--no-color` or `NO_COLOR` to force-disable:To verify, please confirm stdout TTY checks are applied in the pager/color initialization paths.
200-215: Color precedence and portability look good.Clear precedence of NO_COLOR over COLOR and dual support of ATMOS_* is well stated.
225-233: Portability section is solid.Good explanation of dual env var support.
77-83: Documentation confirmed for--redirect-stderr: the flag is registered globally on RootCmd (cmd/root.go) with semantics matching the docs (supports any file path or standard file descriptor).
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/validate_editorconfig.go (1)
101-113: Fix format flag shadowing and wrong error valueThe local
format := outputformat.OutputFormat(format)shadows the package-levelformatand the error path reports the config value instead of the flag value.Apply:
- format := outputformat.OutputFormat(atmosConfig.Validate.EditorConfig.Format) - if ok := format.IsValid(); !ok { + cfgFmt := outputformat.OutputFormat(atmosConfig.Validate.EditorConfig.Format) + if ok := cfgFmt.IsValid(); !ok { errUtils.CheckErrorPrintAndExit(fmt.Errorf("%v is not a valid format choose from the following: %v", atmosConfig.Validate.EditorConfig.Format, outputformat.GetArgumentChoiceText()), "", "") } - cliConfig.Format = format + cliConfig.Format = cfgFmt ... -} else if cmd.Flags().Changed("format") { - format := outputformat.OutputFormat(format) - if ok := format.IsValid(); !ok { - errUtils.CheckErrorPrintAndExit(fmt.Errorf("%v is not a valid format choose from the following: %v", atmosConfig.Validate.EditorConfig.Format, outputformat.GetArgumentChoiceText()), "", "") - } - cliConfig.Format = format +} else if cmd.Flags().Changed("format") { + flagFmt := outputformat.OutputFormat(format) + if ok := flagFmt.IsValid(); !ok { + errUtils.CheckErrorPrintAndExit(fmt.Errorf("%v is not a valid format choose from the following: %v", format, outputformat.GetArgumentChoiceText()), "", "") + } + cliConfig.Format = flagFmt }
♻️ Duplicate comments (1)
pkg/config/config_test.go (1)
669-870: Don’t reimplement env→config merge in tests; exercise shared helper and add more casesThese tests manually mirror env binding (BindEnv + direct struct writes). This can drift from prod behavior. Prefer a shared helper used by both prod and tests, or call the actual init path.
Also, consider cases for boolean synonyms (on/off/yes/no/TRUE/FALSE) for pager and color to cover “smart parsing,” and use t.Setenv for brevity.
Apply (example adjustments):
- // Apply environment variables to config - if envPager := v.GetString("settings.terminal.pager"); envPager != "" { - atmosConfig.Settings.Terminal.Pager = envPager - } - if v.IsSet("settings.terminal.no_color") { - atmosConfig.Settings.Terminal.NoColor = v.GetBool("settings.terminal.no_color") - if atmosConfig.Settings.Terminal.NoColor { - atmosConfig.Settings.Terminal.Color = false - } - } - if v.IsSet("settings.terminal.color") && !atmosConfig.Settings.Terminal.NoColor { - atmosConfig.Settings.Terminal.Color = v.GetBool("settings.terminal.color") - } + // Prefer invoking the real merge/binding path or a shared helper to avoid duplication: + // e.g., bindEnvAndMergeIntoConfig(v, atmosConfig)Optionally, add table rows for:
- PAGER=on/off, ATMOS_PAGER=on/off
- ATMOS_COLOR=on/TRUE vs NO_COLOR=0/false
- Mixed-case inputs.
And replace manual env save/restore with t.Setenv.
🧹 Nitpick comments (7)
website/docs/cli/global-flags.mdx (4)
66-82: Tighten pager wording.Clarify “default settings” to “configured/default pager.”
- - `--pager` (no value): Enable pager with default settings + - `--pager` (no value): Enable pager using the configured/default pager
321-322: Fix misleading comment about PAGER=cat.It doesn’t disable the pager; it effectively bypasses pagination.
-export PAGER=cat # Disables paging in Atmos and other tools that respect PAGER +export PAGER=cat # Effectively disables pagination by piping output through cat
63-64: Use proper Markdown links for no-color.org.Improves rendering and consistency.
- - The `NO_COLOR` env var follows the standard from https://no-color.org/ + - The `NO_COLOR` env var follows the standard from [no-color.org](https://no-color.org/)- - `NO_COLOR` is a standard environment variable supported by many CLI tools (https://no-color.org/) + - `NO_COLOR` is a standard environment variable supported by many CLI tools ([no-color.org](https://no-color.org/))Also applies to: 218-219
329-336: Clarify pipe behavior precedence.Make explicit that piping overrides flag/env.
-When output is piped to another command, Atmos automatically disables color output and pager to ensure compatibility: +When output is piped to another command, Atmos automatically disables color output and pager (even if enabled via flag/env) to ensure compatibility:internal/exec/describe_component_test.go (3)
43-73: Drop duplicate “(legacy test)” cases and align namesThe YAML/JSON “(legacy test)” entries duplicate the preceding two cases and add noise. If no legacy-specific code path exists anymore, remove them.
- { - name: "Test with YAML format (legacy test)", - params: DescribeComponentParams{ - Component: "component-1", - Stack: "nonprod", - Format: "yaml", - }, - }, - { - name: "Test with JSON format (legacy test)", - params: DescribeComponentParams{ - Component: "component-1", - Stack: "nonprod", - Format: "json", - }, - },
15-15: Rename test: it no longer validates pager behaviorPager is global now. Suggest renaming for accuracy.
-func TestExecuteDescribeComponentCmd_Success_YAMLWithPager(t *testing.T) { +func TestExecuteDescribeComponentCmd_Success_Formats(t *testing.T) {
111-115: Assert the concrete error type for invalid formatStrengthen the check with ErrorAs to ensure DescribeConfigFormatError is returned.
- if test.expectedError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + if test.expectedError { + var ferr DescribeConfigFormatError + assert.Error(t, err) + assert.ErrorAs(t, err, &ferr) + } else { + assert.NoError(t, err) + }
📜 Review details
Configuration used: Path: .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 (9)
cmd/describe_component.go(0 hunks)cmd/root.go(3 hunks)cmd/validate_editorconfig.go(1 hunks)internal/exec/describe_component.go(0 hunks)internal/exec/describe_component_test.go(2 hunks)pkg/config/config.go(1 hunks)pkg/config/config_test.go(1 hunks)pkg/schema/schema.go(2 hunks)website/docs/cli/global-flags.mdx(1 hunks)
💤 Files with no reviewable changes (2)
- internal/exec/describe_component.go
- cmd/describe_component.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/config/config.go
- pkg/schema/schema.go
- cmd/root.go
🧰 Additional context used
📓 Path-based instructions (4)
cmd/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/*.go: Implement each Cobra command in a separate file under the cmd/ directory
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in command help
Provide meaningful feedback to users in command implementation
Include progress indicators for long-running operations
Provide clear error messages to users
Include troubleshooting hints when appropriate
Log detailed errors for debugging
Files:
cmd/validate_editorconfig.go
**/*.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:
cmd/validate_editorconfig.gopkg/config/config_test.gointernal/exec/describe_component_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
Files:
pkg/config/config_test.gointernal/exec/describe_component_test.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in the website/ directory when adding new features
Follow the website's documentation structure and style
Keep website code in the website/ directory
Follow the existing website architecture and style
Document new features on the website
Include examples and use cases in website documentation
Files:
website/docs/cli/global-flags.mdx
🧠 Learnings (19)
📚 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:
cmd/validate_editorconfig.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
PR: cloudposse/atmos#736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
cmd/validate_editorconfig.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
PR: cloudposse/atmos#808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
cmd/validate_editorconfig.go
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Applied to files:
cmd/validate_editorconfig.go
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
PR: cloudposse/atmos#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:
cmd/validate_editorconfig.go
📚 Learning: 2025-01-30T16:56:43.004Z
Learnt from: mcalhoun
PR: cloudposse/atmos#980
File: pkg/utils/log_utils.go:62-63
Timestamp: 2025-01-30T16:56:43.004Z
Learning: In pkg/utils/log_utils.go, LogTrace is intentionally redirected to LogDebug since charmbracelet logger doesn't support Trace level, maintaining backward compatibility with the original LogTrace functionality.
Applied to files:
cmd/validate_editorconfig.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
cmd/validate_editorconfig.go
📚 Learning: 2024-11-10T19:28:17.365Z
Learnt from: osterman
PR: cloudposse/atmos#768
File: internal/exec/vendor_utils.go:0-0
Timestamp: 2024-11-10T19:28:17.365Z
Learning: When TTY is not supported, log the downgrade message at the Warn level using `u.LogWarning(cliConfig, ...)` instead of `fmt.Println`.
Applied to files:
cmd/validate_editorconfig.go
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
cmd/validate_editorconfig.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
PR: cloudposse/atmos#1202
File: pkg/utils/yaml_func_exec.go:104-104
Timestamp: 2025-04-23T15:02:50.246Z
Learning: In the Atmos codebase, direct calls to `os.Getenv` should be avoided. Instead, use `viper.BindEnv` for environment variable access. This provides a consistent approach to configuration management across the codebase.
Applied to files:
pkg/config/config_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
PR: cloudposse/atmos#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/config/config_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Consider using testify/mock for creating mock implementations
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
PR: cloudposse/atmos#1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Include integration tests for command flows
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2024-12-12T17:13:53.409Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#848
File: pkg/utils/doc_utils.go:19-22
Timestamp: 2024-12-12T17:13:53.409Z
Learning: In `pkg/utils/doc_utils.go`, the `DisplayDocs` function uses the `PAGER` environment variable, which is intentionally user-configurable to allow users to specify custom pager commands that fit their workflow; adding validation to restrict it is not desired.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
website/docs/cli/global-flags.mdx
🧬 Code graph analysis (1)
pkg/config/config_test.go (1)
pkg/schema/schema.go (4)
AtmosConfiguration(26-61)Settings(853-857)AtmosSettings(245-265)Terminal(204-212)
🪛 LanguageTool
website/docs/cli/global-flags.mdx
[grammar] ~7-~7: There might be a mistake here.
Context: ...Intro from '@site/src/components/Intro'; import Note from '@site/src/components/N...
(QB_NEW_EN)
[grammar] ~190-~190: There might be a mistake here.
Context: ...1. Command-line flags (highest priority) 2. Environment variables 3. Configuration f...
(QB_NEW_EN)
[grammar] ~191-~191: There might be a mistake here.
Context: ...ghest priority) 2. Environment variables 3. Configuration file (atmos.yaml) 4. Def...
(QB_NEW_EN)
[grammar] ~192-~192: There might be a mistake here.
Context: ...les 3. Configuration file (atmos.yaml) 4. Default values (lowest priority) ### Co...
(QB_NEW_EN)
⏰ 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: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (5)
website/docs/cli/global-flags.mdx (2)
72-78: Nice: command-with-args support is clearly documented.Explicitly calling out “any non-boolean value is a command” plus quoting examples matches how users expect to pass pager args.
16-20: Global flags wiring and defaults match documentation.cmd/validate_editorconfig.go (1)
114-120: Good: use constant for trace levelReplacing the string literal with
u.LogLevelTraceavoids typos and centralizes semantics.pkg/config/config_test.go (2)
531-611: Solid pager/no-color flag parsing coverageGood table-driven tests validating --pager forms and interaction with --no-color.
613-667: Verify NoColor effects; looks correctAsserting Color=false when --no-color is set matches schema semantics.
github-actions
bot
added
size/xl
|
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
cmd/packer_output_test.go (1)
20-24: Restore logger output to avoid leaking test state.
log.SetOutput(w)isn’t reverted; subsequent tests may write to a closed pipe.// Restore std - err = w.Close() + // Restore logger output before closing the pipe to avoid writes to a closed fd + log.SetOutput(os.Stderr) + err = w.Close() assert.NoError(t, err) os.Stdout = oldStdAlso applies to: 29-33
internal/exec/packer_test.go (3)
34-39: Reset logger output after capture in Validate test.
log.SetOutput(w)isn’t restored; can affect other tests.os.Stdout = w log.SetOutput(w) packerFlags := PackerFlags{} err := ExecutePacker(&info, &packerFlags) assert.NoError(t, err) // Restore std - err = w.Close() + // Restore logger output before closing the writer + log.SetOutput(os.Stderr) + err = w.Close() assert.NoError(t, err) os.Stdout = oldStdAlso applies to: 45-48
82-88: Reset logger output after capture in Inspect test.Same leak risk here.
os.Stdout = w log.SetOutput(w) packerFlags := PackerFlags{} err := ExecutePacker(&info, &packerFlags) assert.NoError(t, err) // Restore std - err = w.Close() + log.SetOutput(os.Stderr) + err = w.Close() assert.NoError(t, err) os.Stdout = oldStdAlso applies to: 92-96
124-131: Reset logger output after capture in Version test.Restore logger output to default.
os.Stdout = w log.SetOutput(w) packerFlags := PackerFlags{} err := ExecutePacker(&info, &packerFlags) assert.NoError(t, err) // Restore std - err = w.Close() + log.SetOutput(os.Stderr) + err = w.Close() assert.NoError(t, err) os.Stdout = oldStdAlso applies to: 134-138
pkg/config/default.go (1)
63-67: Disable default pager: update pkg/config/default.go (line 65) toPager: ""and change in pkg/config/load.go (line 139)v.SetDefault("settings.terminal.pager", true)→falseto align with disabled-by-default intent.cmd/root_test.go (1)
36-39: Restore logger output after test.Prevent global logger from remaining pointed at the buffer.
// Create a buffer to capture the output var buf bytes.Buffer log.SetOutput(&buf) + defer log.SetOutput(os.Stderr)Also applies to: 55-56
internal/exec/describe_component_test.go (1)
38-41: Actually assert yq filtering (and cover no-pager + query).The “query” scenario doesn’t validate filtering:
evaluateYqExpressionreturns the unmodified map and pager content isn’t asserted. Add a per-subtest yq stub and assert filtered data when pager is off. Also makeprintOrWriteToFilequery-aware.Apply:
@@ - evaluateYqExpression: func(atmosConfig *schema.AtmosConfiguration, data any, yq string) (any, error) { - return data, nil - }, + evaluateYqExpression: func(atmosConfig *schema.AtmosConfiguration, data any, yq string) (any, error) { + return data, nil + }, @@ - if test.expectPager { + if test.expectPager { mockPager := pager.NewMockPageCreator(ctrl) if test.expectedError && test.params.Format == "invalid-format" { // The pager won't be called because viewConfig will return error before reaching pageCreator } else { mockPager.EXPECT().Run("component-1", gomock.Any()).Return(nil).Times(1) } mockedExec.pageCreator = mockPager } else { // For non-pager tests, we don't need to mock the pager as it won't be called mockedExec.pageCreator = nil } @@ - // Mock the initCliConfig to return a config with the test's pager setting + // Mock the initCliConfig to return a config with the test's pager setting mockedExec.initCliConfig = func(configAndStacksInfo schema.ConfigAndStacksInfo, processStacks bool) (schema.AtmosConfiguration, error) { return schema.AtmosConfiguration{ Settings: schema.AtmosSettings{ Terminal: schema.Terminal{ Pager: test.pagerSetting, }, }, }, nil } + // Override yq evaluator per test: simulate filtering when a query is provided + mockedExec.evaluateYqExpression = func(atmosConfig *schema.AtmosConfiguration, data any, yq string) (any, error) { + if yq != "" { + return "component-1", nil + } + return data, nil + } @@ - // Mock printOrWriteToFile - this should only be called when pager is disabled or fails + // Mock printOrWriteToFile - this should only be called when pager is disabled or fails mockedExec.printOrWriteToFile = func(atmosConfig *schema.AtmosConfiguration, format string, file string, data any) error { printOrWriteToFileCalled = true if test.expectedError && test.params.Format == "invalid-format" { return DescribeConfigFormatError{format: "invalid-format"} } assert.Equal(t, test.params.Format, format) assert.Equal(t, "", file) - assert.Equal(t, map[string]any{ - "component": "component-1", - "stack": "nonprod", - }, data) + if test.params.Query != "" { + assert.Equal(t, "component-1", data) + } else { + assert.Equal(t, map[string]any{ + "component": "component-1", + "stack": "nonprod", + }, data) + } return nil } @@ }, } + // Add coverage for "query without pager" to prove filtered output hits print path + tests = append(tests, struct { + name string + params DescribeComponentParams + pagerSetting string + expectPager bool + expectedError bool + }{ + name: "Test query without pager", + params: DescribeComponentParams{ + Component: "component-1", + Stack: "nonprod", + Format: "json", + Query: ".component", + }, + pagerSetting: "off", + expectPager: false, + })Also applies to: 117-126, 131-141, 142-155, 112-112
🧹 Nitpick comments (5)
cmd/packer_output_test.go (1)
17-19: Align log level with new default.Env sets "Warning" but the logger is set to Info. Switch to Warn to avoid confusion.
- log.SetLevel(log.InfoLevel) + log.SetLevel(log.WarnLevel)internal/exec/packer_test.go (1)
21-22: Optional: make logger level match env (Warn).Minor consistency tweak.
- log.SetLevel(log.InfoLevel) + log.SetLevel(log.WarnLevel)Also applies to: 69-70, 117-118, 159-160, 183-184
cmd/root_test.go (1)
22-24: Optional: drop the initial Warning set.This is overridden by
t.Setenv("ATMOS_LOGS_LEVEL", "Debug")later; the early set is redundant.- err = os.Setenv("ATMOS_LOGS_LEVEL", "Warning") - assert.NoError(t, err, "Setting 'ATMOS_LOGS_LEVEL' environment variable should execute without error")internal/exec/describe_component_test.go (1)
17-21: Rename test to reflect the matrix it runs.Covers YAML/JSON, pager on/off, errors—not just “YAMLWithPager”.
-func TestExecuteDescribeComponentCmd_Success_YAMLWithPager(t *testing.T) { +func TestExecuteDescribeComponentCmd_PagerMatrix(t *testing.T) {website/docs/cli/global-flags.mdx (1)
66-72: Tighten wording: “configured/default pager”.Make it explicit that
--pagerwith no value uses the configured (or default) pager command.- - `--pager` (no value): Enable pager with default settings + - `--pager` (no value): Enable pager using the configured/default pager
📜 Review details
Configuration used: Path: .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 (14)
cmd/packer_init_test.go(1 hunks)cmd/packer_inspect_test.go(1 hunks)cmd/packer_output_test.go(1 hunks)cmd/packer_validate_test.go(1 hunks)cmd/packer_version_test.go(1 hunks)cmd/root.go(3 hunks)cmd/root_test.go(1 hunks)internal/exec/describe_component_test.go(2 hunks)internal/exec/packer_output_test.go(1 hunks)internal/exec/packer_test.go(5 hunks)pkg/config/default.go(1 hunks)pkg/config/load.go(2 hunks)pkg/config/load_config_test.go(3 hunks)website/docs/cli/global-flags.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/config/load.go
- cmd/root.go
🧰 Additional context used
📓 Path-based instructions (4)
cmd/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/*.go: Implement each Cobra command in a separate file under the cmd/ directory
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in command help
Provide meaningful feedback to users in command implementation
Include progress indicators for long-running operations
Provide clear error messages to users
Include troubleshooting hints when appropriate
Log detailed errors for debugging
Files:
cmd/packer_output_test.gocmd/root_test.gocmd/packer_init_test.gocmd/packer_version_test.gocmd/packer_inspect_test.gocmd/packer_validate_test.go
**/*.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:
cmd/packer_output_test.gointernal/exec/packer_output_test.gocmd/root_test.gopkg/config/default.gocmd/packer_init_test.gocmd/packer_version_test.gopkg/config/load_config_test.gocmd/packer_inspect_test.gointernal/exec/packer_test.gocmd/packer_validate_test.gointernal/exec/describe_component_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
Files:
cmd/packer_output_test.gointernal/exec/packer_output_test.gocmd/root_test.gocmd/packer_init_test.gocmd/packer_version_test.gopkg/config/load_config_test.gocmd/packer_inspect_test.gointernal/exec/packer_test.gocmd/packer_validate_test.gointernal/exec/describe_component_test.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in the website/ directory when adding new features
Follow the website's documentation structure and style
Keep website code in the website/ directory
Follow the existing website architecture and style
Document new features on the website
Include examples and use cases in website documentation
Files:
website/docs/cli/global-flags.mdx
🧠 Learnings (20)
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
cmd/packer_output_test.gointernal/exec/packer_output_test.gocmd/packer_init_test.gocmd/packer_version_test.gocmd/packer_inspect_test.gointernal/exec/packer_test.gocmd/packer_validate_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
cmd/packer_output_test.gointernal/exec/packer_output_test.gocmd/root_test.gocmd/packer_init_test.gocmd/packer_version_test.gocmd/packer_inspect_test.gointernal/exec/packer_test.gocmd/packer_validate_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#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:
cmd/packer_output_test.gointernal/exec/packer_output_test.gocmd/packer_version_test.gocmd/packer_inspect_test.gointernal/exec/packer_test.gocmd/packer_validate_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests. This is implemented through custom testing framework extensions and is used consistently throughout the test suite.
Applied to files:
cmd/packer_output_test.gointernal/exec/packer_output_test.gocmd/packer_version_test.gocmd/packer_inspect_test.gointernal/exec/packer_test.gocmd/packer_validate_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#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:
cmd/packer_output_test.gointernal/exec/packer_output_test.gocmd/packer_version_test.gocmd/packer_inspect_test.gointernal/exec/packer_test.gocmd/packer_validate_test.go
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Applied to files:
cmd/packer_output_test.gocmd/packer_version_test.gopkg/config/load_config_test.gocmd/packer_inspect_test.gocmd/packer_validate_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
internal/exec/packer_output_test.gointernal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios
Applied to files:
internal/exec/packer_output_test.gointernal/exec/describe_component_test.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/packer_output_test.gocmd/root_test.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
cmd/root_test.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
PR: cloudposse/atmos#1202
File: pkg/utils/yaml_func_exec.go:104-104
Timestamp: 2025-04-23T15:02:50.246Z
Learning: In the Atmos codebase, direct calls to `os.Getenv` should be avoided. Instead, use `viper.BindEnv` for environment variable access. This provides a consistent approach to configuration management across the codebase.
Applied to files:
cmd/root_test.go
📚 Learning: 2024-11-10T19:28:17.365Z
Learnt from: osterman
PR: cloudposse/atmos#768
File: internal/exec/vendor_utils.go:0-0
Timestamp: 2024-11-10T19:28:17.365Z
Learning: When TTY is not supported, log the downgrade message at the Warn level using `u.LogWarning(cliConfig, ...)` instead of `fmt.Println`.
Applied to files:
pkg/config/default.gopkg/config/load_config_test.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
cmd/packer_init_test.go
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
cmd/packer_version_test.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
PR: cloudposse/atmos#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_test.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
PR: cloudposse/atmos#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_test.go
📚 Learning: 2024-12-12T17:13:53.409Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#848
File: pkg/utils/doc_utils.go:19-22
Timestamp: 2024-12-12T17:13:53.409Z
Learning: In `pkg/utils/doc_utils.go`, the `DisplayDocs` function uses the `PAGER` environment variable, which is intentionally user-configurable to allow users to specify custom pager commands that fit their workflow; adding validation to restrict it is not desired.
Applied to files:
website/docs/cli/global-flags.mdxinternal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use test fixtures for complex inputs
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2025-01-19T23:13:50.429Z
Learnt from: aknysh
PR: cloudposse/atmos#943
File: internal/exec/describe_config.go:37-37
Timestamp: 2025-01-19T23:13:50.429Z
Learning: When reviewing pointer usage with `EvaluateYqExpression`, check if the `atmosConfig` parameter is already a pointer (e.g., when returned by `InitCliConfig`) to avoid suggesting unnecessary address-of operations.
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Consider using testify/mock for creating mock implementations
Applied to files:
internal/exec/describe_component_test.go
🧬 Code graph analysis (1)
internal/exec/describe_component_test.go (4)
internal/exec/describe_component.go (1)
DescribeComponentParams(14-23)pkg/pager/mock_pager.go (1)
NewMockPageCreator(25-29)pkg/schema/schema.go (4)
AtmosConfiguration(26-61)Settings(853-857)AtmosSettings(245-265)Terminal(204-212)internal/exec/describe_config.go (1)
DescribeConfigFormatError(16-18)
🪛 LanguageTool
website/docs/cli/global-flags.mdx
[grammar] ~7-~7: There might be a mistake here.
Context: ...Intro from '@site/src/components/Intro'; import Note from '@site/src/components/N...
(QB_NEW_EN)
[grammar] ~151-~151: There might be a mistake here.
Context: ...1. Command-line flags (highest priority) 2. Environment variables 3. Configuration f...
(QB_NEW_EN)
[grammar] ~152-~152: There might be a mistake here.
Context: ...ghest priority) 2. Environment variables 3. Configuration file (atmos.yaml) 4. Def...
(QB_NEW_EN)
[grammar] ~153-~153: There might be a mistake here.
Context: ...les 3. Configuration file (atmos.yaml) 4. Default values (lowest priority) ### Co...
(QB_NEW_EN)
⏰ 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). (11)
- GitHub Check: [mock-macos] tests/fixtures/scenarios/complete
- GitHub Check: [mock-windows] examples/demo-atlantis
- GitHub Check: [mock-windows] examples/demo-component-versions
- GitHub Check: [mock-windows] tests/fixtures/scenarios/complete
- GitHub Check: [mock-windows] examples/demo-vendoring
- GitHub Check: [k3s] demo-helmfile
- GitHub Check: [localstack] demo-localstack
- GitHub Check: Acceptance Tests (macos-latest, macos)
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Acceptance Tests (ubuntu-latest, linux)
- GitHub Check: Summary
🔇 Additional comments (5)
internal/exec/packer_output_test.go (1)
18-18: LGTM: test updated to Warning levelMatches the new default log-level semantics. No other concerns here.
internal/exec/packer_test.go (1)
20-21: LGTM: tests updated to Warning level.Matches the new default and keeps expectations consistent.
Also applies to: 68-69, 116-118, 158-160, 182-184
pkg/config/default.go (1)
81-83: LGTM: default log level → Warning.Consistent with updated tests and reduced verbosity.
pkg/config/load_config_test.go (1)
31-32: LGTM: expectations updated to Warning.Matches the new default and validates merge precedence correctly.
Also applies to: 83-84, 102-103
internal/exec/describe_component_test.go (1)
117-141: Nice: clean pager mocking and global config injection.Good use of gomock and driving behavior via
Terminal.Pager. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/root.go (1)
290-294: Avoid import shadowing: rename local pager var.Variable pager shadows imported package pager; revive/import-shadowing may flag this.
- pager := pager.NewWithAtmosConfig(pagerEnabled) - if err := pager.Run("Atmos CLI Help", buf.String()); err != nil { + pg := pager.NewWithAtmosConfig(pagerEnabled) + if err := pg.Run("Atmos CLI Help", buf.String()); err != nil { log.Error("Failed to run pager", "error", err) utils.OsExit(1) }
🧹 Nitpick comments (12)
pkg/pager/pager_test.go (5)
97-100: Make option-count assertion resilient.Len==1 is brittle if more ProgramOptions are added later. Prefer “at least one” to keep intent while avoiding churn.
Apply this diff:
- assert.NotNil(t, capturedModel) - assert.Len(t, capturedOpts, 1, "Should have 1 tea program option (WithAltScreen)") + assert.NotNil(t, capturedModel) + assert.GreaterOrEqual(t, len(capturedOpts), 1, "At least 1 tea program option expected (e.g., WithAltScreen).")
176-190: Broaden constructor invariants.Also assert core function fields are set for both enabled/disabled cases to catch regressions.
Apply this diff:
func TestNewWithAtmosConfig(t *testing.T) { // Test with pager enabled pcEnabled := NewWithAtmosConfig(true) assert.NotNil(t, pcEnabled) concretePC, ok := pcEnabled.(*pageCreator) assert.True(t, ok) assert.True(t, concretePC.enablePager, "Pager should be enabled when true is passed") + assert.NotNil(t, concretePC.newTeaProgram) + assert.NotNil(t, concretePC.contentFitsTerminal) + assert.NotNil(t, concretePC.isTTYSupportForStdout) // Test with pager disabled pcDisabled := NewWithAtmosConfig(false) assert.NotNil(t, pcDisabled) concretePC2, ok := pcDisabled.(*pageCreator) assert.True(t, ok) assert.False(t, concretePC2.enablePager, "Pager should be disabled when false is passed") + assert.NotNil(t, concretePC2.newTeaProgram) + assert.NotNil(t, concretePC2.contentFitsTerminal) + assert.NotNil(t, concretePC2.isTTYSupportForStdout) }
228-230: Mirror the resilient assertion here too.Same brittleness; prefer minimum count.
Apply this diff:
- // Verify the program options - assert.Len(t, capturedOpts, 1, "Should have 1 tea program option (WithAltScreen)") + // Verify the program options. + assert.GreaterOrEqual(t, len(capturedOpts), 1, "At least 1 tea program option expected (e.g., WithAltScreen).")
236-245: Good coverage of “pager not used” branches.Cases for disabled pager, no TTY, and content fits look solid. Consider running subtests in parallel to speed up CI (safe here since dependencies are stubbed).
Apply this diff:
t.Run(tc.name, func(t *testing.T) { + t.Parallel() teaProgramCalled := falseAlso applies to: 248-251, 255-255, 265-265
70-71: Fix godot lints: end comments with periods.A few comments lack trailing periods; golangci-lint godot will flag them.
Apply this diff:
- // Create a real tea.Program but with a simple model that won't actually run - // Since we're mocking the function, we control what gets returned + // Create a real tea.Program but with a simple model that won't actually run. + // Since we're mocking the function, we control what gets returned. - // Note: In practice, tea.Program.Run() doesn't typically return errors from Update() - // This is more for demonstrating how we might test error scenarios + // Note: In practice, tea.Program.Run() doesn't typically return errors from Update(). + // This is more for demonstrating how we might test error scenarios.Also applies to: 139-140
pkg/schema/schema.go (1)
209-212: Fix godot linter: end comments with periods.Several new/edited comments lack trailing periods.
Apply:
type Terminal struct { @@ - NoColor bool `yaml:"no_color" json:"no_color" mapstructure:"no_color"` // Deprecated in config, use Color instead + NoColor bool `yaml:"no_color" json:"no_color" mapstructure:"no_color"` // Deprecated in config, use Color instead. } func (t *Terminal) IsPagerEnabled() bool { - // Changed: pager is DISABLED by default - // Only enabled if explicitly set to true/on/yes/1 or a pager command + // Changed: pager is DISABLED by default. + // Only enabled if explicitly set to true/on/yes/1 or a pager command. @@ - // Enable for true/on/yes/1 or specific pager commands like "less", "more" + // Enable for true/on/yes/1 or specific pager commands like "less", "more". @@ - // Check deprecated NoColor field for backward compatibility + // Check deprecated NoColor field for backward compatibility. @@ - // Use Color setting (defaults to true if not explicitly set) + // Use Color setting (defaults to true if not explicitly set).Also applies to: 215-216, 221-221, 228-228, 231-231
pkg/schema/schema_test.go (1)
40-62: Use testify consistently for clarity.Switch t.Errorf to assert.Equal for consistency with the rest of the file.
- result := term.IsColorEnabled() - if result != tt.expect { - t.Errorf("IsColorEnabled() for Color=%v, NoColor=%v: expected %v, got %v", tt.color, tt.noColor, tt.expect, result) - } + result := term.IsColorEnabled() + assert.Equal(t, tt.expect, result, "IsColorEnabled() mismatch for Color=%v, NoColor=%v", tt.color, tt.noColor)cmd/root.go (1)
273-289: Fix godot linter: end nearby comments with periods.A few new comments miss trailing periods.
- // Set NoOptDefVal so --pager without value means "true" + // Set NoOptDefVal so --pager without value means "true". @@ - // Check if pager should be enabled based on flag, env var, or config + // Check if pager should be enabled based on flag, env var, or config. @@ - // Check if --pager flag was explicitly set + // Check if --pager flag was explicitly set. @@ - // Handle --pager flag values using switch for better readability + // Handle --pager flag values using switch for better readability. @@ - // Assume it's a pager command like "less" or "more" + // Assume it's a pager command like "less" or "more".Also applies to: 212-214
pkg/config/config.go (1)
80-91: Fix godot linter: end comments with periods.Trailing periods missing in several new comments.
- // If value is neither "true" nor "false", leave defaults unchanged + // If value is neither "true" nor "false", leave defaults unchanged. @@ - // Handle --pager global flag + // Handle --pager global flag. @@ - // Handle NO_PAGER environment variable (standard CLI convention) - // Check this after --pager flag so CLI flag takes precedence + // Handle NO_PAGER environment variable (standard CLI convention). + // Check this after --pager flag so CLI flag takes precedence. @@ - // Check if --pager flag was explicitly provided + // Check if --pager flag was explicitly provided. @@ - // NO_PAGER is set and no explicit --pager flag was provided, disable the pager + // NO_PAGER is set and no explicit --pager flag was provided, disable the pager.Also applies to: 92-96, 97-106
internal/exec/describe_component_test.go (1)
18-21: Tiny cleanup: use t.Cleanup for gomock controller.Keeps teardown consistent with other tests.
- ctrl := gomock.NewController(t) - defer ctrl.Finish() + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish)pkg/config/config_test.go (2)
381-416: Assert NoColor in TestSetLogConfig; fix typo.The expectetdNoColor field is misspelled and unused; add assertion to verify NoColor behavior.
- expectetdNoColor bool + expectedNoColor bool @@ - expectetdNoColor: true, + expectedNoColor: true, @@ - expectetdNoColor: true, + expectedNoColor: true, @@ - expectetdNoColor: false, + expectedNoColor: false, @@ - assert.Equal(t, test.expectedLogLevel, atmosConfig.Logs.Level) + assert.Equal(t, test.expectedLogLevel, atmosConfig.Logs.Level) + if test.args != nil { + assert.Equal(t, test.expectedNoColor, atmosConfig.Settings.Terminal.NoColor) + }
628-879: Solid env precedence coverage; consider t.Setenv for brevity.Good matrix across ATMOS_PAGER/PAGER/NO_PAGER and color envs with flag overrides. Optionally replace manual save/restore with t.Setenv.
📜 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 (32)
atmos.yaml(1 hunks)cmd/describe.go(0 hunks)cmd/describe_affected.go(1 hunks)cmd/describe_component.go(0 hunks)cmd/describe_config.go(1 hunks)cmd/describe_dependents.go(1 hunks)cmd/describe_stacks.go(1 hunks)cmd/describe_workflows.go(1 hunks)cmd/packer_init_test.go(1 hunks)cmd/packer_inspect_test.go(1 hunks)cmd/packer_output_test.go(1 hunks)cmd/packer_validate_test.go(1 hunks)cmd/packer_version_test.go(1 hunks)cmd/root.go(3 hunks)cmd/root_test.go(1 hunks)cmd/validate_editorconfig.go(1 hunks)internal/exec/describe_component.go(0 hunks)internal/exec/describe_component_test.go(2 hunks)internal/exec/packer_output_test.go(1 hunks)internal/exec/packer_test.go(5 hunks)pkg/config/config.go(1 hunks)pkg/config/config_test.go(2 hunks)pkg/config/default.go(1 hunks)pkg/config/load.go(2 hunks)pkg/config/load_config_test.go(3 hunks)pkg/pager/pager.go(2 hunks)pkg/pager/pager_test.go(4 hunks)pkg/schema/schema.go(2 hunks)pkg/schema/schema_test.go(2 hunks)website/docs/cli/configuration/configuration.mdx(3 hunks)website/docs/cli/configuration/terminal.mdx(3 hunks)website/docs/cli/global-flags.mdx(1 hunks)
💤 Files with no reviewable changes (3)
- cmd/describe.go
- cmd/describe_component.go
- internal/exec/describe_component.go
🚧 Files skipped from review as they are similar to previous changes (21)
- internal/exec/packer_output_test.go
- cmd/packer_version_test.go
- cmd/packer_validate_test.go
- cmd/packer_init_test.go
- cmd/describe_dependents.go
- internal/exec/packer_test.go
- website/docs/cli/global-flags.mdx
- cmd/describe_stacks.go
- cmd/describe_affected.go
- cmd/describe_config.go
- pkg/config/load_config_test.go
- pkg/config/default.go
- website/docs/cli/configuration/configuration.mdx
- cmd/validate_editorconfig.go
- cmd/root_test.go
- pkg/config/load.go
- cmd/packer_inspect_test.go
- cmd/describe_workflows.go
- pkg/pager/pager.go
- atmos.yaml
- cmd/packer_output_test.go
🧰 Additional context used
📓 Path-based instructions (7)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in the website/ directory when adding new features
Follow the website's documentation structure and style
Keep website code in the website/ directory
Follow the existing website architecture and style
Document new features on the website
Include examples and use cases in website documentation
Files:
website/docs/cli/configuration/terminal.mdx
**/*.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
**/*.go: All comments in Go code must end with periods (enforced by golangci-lint godot).
Wrap all returned errors using static errors from the errors package; never return dynamic errors directly.
Always bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for each external var.
Separate structured logging from UI output: use stderr for prompts/errors to user; stdout only for data; never use logging for UI.
Most text UI must go to stderr; only data/results to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
For non-standard execution paths, capture telemetry with telemetry.CaptureCmd or telemetry.CaptureCmdString and never capture user data.
Ensure cross-platform compatibility: prefer SDKs to external binaries, use filepath/os/runtime for portability, and add build constraints when needed.
Files:
pkg/schema/schema_test.gopkg/config/config.gopkg/pager/pager_test.gocmd/root.gointernal/exec/describe_component_test.gopkg/schema/schema.gopkg/config/config_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
**/*_test.go: Always use t.Skipf() with a reason for skipped tests; never use t.Skip() or reasonless t.Skipf().
Test files must mirror implementation structure and naming (e.g., aws_ssm_store_test.go).
Use table-driven unit tests and mock interfaces for external dependencies; target >80% coverage.
Files:
pkg/schema/schema_test.gopkg/pager/pager_test.gointernal/exec/describe_component_test.gopkg/config/config_test.go
pkg/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests reside under pkg/ alongside implementations.
Files:
pkg/schema/schema_test.gopkg/pager/pager_test.gopkg/config/config_test.go
cmd/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/*.go: Implement each Cobra command in a separate file under the cmd/ directory
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in command help
Provide meaningful feedback to users in command implementation
Include progress indicators for long-running operations
Provide clear error messages to users
Include troubleshooting hints when appropriate
Log detailed errors for debugging
Files:
cmd/root.go
cmd/**/!(*_test).go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/**/!(*_test).go: One Cobra command per file in cmd/.
Use //go:embed to load *_usage.md examples and utils.PrintfMarkdown() to render them in commands.
Files:
cmd/root.go
cmd/root.go
📄 CodeRabbit inference engine (CLAUDE.md)
New Cobra commands get telemetry via RootCmd.ExecuteC() in cmd/root.go: ensure commands are wired to RootCmd.
Files:
cmd/root.go
🧠 Learnings (25)
📚 Learning: 2025-02-20T13:57:36.326Z
Learnt from: osterman
PR: cloudposse/atmos#1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands supports -1 for unlimited width, which bypasses terminal width restrictions and may require horizontal scrolling. The default is 10 columns if not specified.
Applied to files:
website/docs/cli/configuration/terminal.mdx
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios
Applied to files:
pkg/schema/schema_test.gopkg/pager/pager_test.gointernal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
pkg/schema/schema_test.gointernal/exec/describe_component_test.gopkg/config/config_test.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
pkg/config/config.gopkg/schema/schema.go
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
pkg/config/config.gocmd/root.gopkg/schema/schema.go
📚 Learning: 2024-12-12T17:13:53.409Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#848
File: pkg/utils/doc_utils.go:19-22
Timestamp: 2024-12-12T17:13:53.409Z
Learning: In `pkg/utils/doc_utils.go`, the `DisplayDocs` function uses the `PAGER` environment variable, which is intentionally user-configurable to allow users to specify custom pager commands that fit their workflow; adding validation to restrict it is not desired.
Applied to files:
pkg/config/config.gocmd/root.gointernal/exec/describe_component_test.gopkg/schema/schema.gopkg/config/config_test.go
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
pkg/config/config.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
PR: cloudposse/atmos#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/config.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Follow the precedence order: flags > environment variables > config file > defaults
Applied to files:
pkg/config/config.gopkg/config/config_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use Viper for managing configuration, environment variables, and flags
Applied to files:
pkg/config/config.gopkg/config/config_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Support configuration via files, environment variables, and flags
Applied to files:
pkg/config/config.go
📚 Learning: 2025-09-07T15:13:17.831Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T15:13:17.831Z
Learning: Configuration precedence: CLI flags → ENV vars → config files → defaults using Viper.
Applied to files:
pkg/config/config.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to cmd/*.go : Provide comprehensive help text for all commands and flags
Applied to files:
cmd/root.go
📚 Learning: 2024-11-10T19:28:17.365Z
Learnt from: osterman
PR: cloudposse/atmos#768
File: internal/exec/vendor_utils.go:0-0
Timestamp: 2024-11-10T19:28:17.365Z
Learning: When TTY is not supported, log the downgrade message at the Warn level using `u.LogWarning(cliConfig, ...)` instead of `fmt.Println`.
Applied to files:
cmd/root.go
📚 Learning: 2024-12-11T18:46:02.483Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/terraform.go:39-39
Timestamp: 2024-12-11T18:46:02.483Z
Learning: `cliConfig` is initialized in `cmd/root.go` and can be used across the `cmd` package.
Applied to files:
cmd/root.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-07T15:13:17.831Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T15:13:17.831Z
Learning: Applies to **/*_test.go : Use table-driven unit tests and mock interfaces for external dependencies; target >80% coverage.
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use test fixtures for complex inputs
Applied to files:
internal/exec/describe_component_test.gopkg/config/config_test.go
📚 Learning: 2025-01-19T23:13:50.429Z
Learnt from: aknysh
PR: cloudposse/atmos#943
File: internal/exec/describe_config.go:37-37
Timestamp: 2025-01-19T23:13:50.429Z
Learning: When reviewing pointer usage with `EvaluateYqExpression`, check if the `atmosConfig` parameter is already a pointer (e.g., when returned by `InitCliConfig`) to avoid suggesting unnecessary address-of operations.
Applied to files:
internal/exec/describe_component_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Consider using testify/mock for creating mock implementations
Applied to files:
internal/exec/describe_component_test.gopkg/config/config_test.go
📚 Learning: 2025-09-07T15:13:17.831Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T15:13:17.831Z
Learning: Applies to **/*.go : Always bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for each external var.
Applied to files:
pkg/config/config_test.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
PR: cloudposse/atmos#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/config_test.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
PR: cloudposse/atmos#1202
File: pkg/utils/yaml_func_exec.go:104-104
Timestamp: 2025-04-23T15:02:50.246Z
Learning: In the Atmos codebase, direct calls to `os.Getenv` should be avoided. Instead, use `viper.BindEnv` for environment variable access. This provides a consistent approach to configuration management across the codebase.
Applied to files:
pkg/config/config_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
PR: cloudposse/atmos#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/config/config_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use Viper for configuration management
Applied to files:
pkg/config/config_test.go
🧬 Code graph analysis (6)
pkg/schema/schema_test.go (1)
pkg/schema/schema.go (1)
Terminal(204-212)
pkg/config/config.go (1)
pkg/schema/schema.go (2)
Settings(853-857)Terminal(204-212)
pkg/pager/pager_test.go (1)
pkg/pager/pager.go (1)
NewWithAtmosConfig(23-27)
cmd/root.go (2)
pkg/schema/schema.go (1)
Terminal(204-212)pkg/pager/pager.go (1)
NewWithAtmosConfig(23-27)
internal/exec/describe_component_test.go (4)
internal/exec/describe_component.go (1)
DescribeComponentParams(14-23)pkg/pager/mock_pager.go (1)
NewMockPageCreator(25-29)pkg/schema/schema.go (4)
AtmosConfiguration(26-61)Settings(853-857)AtmosSettings(245-265)Terminal(204-212)internal/exec/describe_config.go (1)
DescribeConfigFormatError(16-18)
pkg/config/config_test.go (1)
pkg/schema/schema.go (4)
AtmosConfiguration(26-61)Settings(853-857)AtmosSettings(245-265)Terminal(204-212)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (9)
pkg/pager/pager_test.go (1)
173-174: Default disabled state verified.Asserting New() defaults enablePager=false aligns with PR objectives. Nice guard.
pkg/schema/schema.go (1)
214-223: Pager normalization and default‑disabled logic LGTM.Case/whitespace normalization and “falsey set” checks look correct and match tests.
pkg/schema/schema_test.go (1)
64-86: Pager tests LGTM.Covers empty, truthy/falsey, commands, and case-insensitivity.
cmd/root.go (2)
212-214: Global --pager flag wiring LGTM.NoOptDefVal="true" gives expected boolean shorthand behavior.
98-101: Respecting IsColorEnabled for logger styles LGTM.Correctly clears level colors when color is disabled.
Also applies to: 101-111
website/docs/cli/configuration/terminal.mdx (1)
36-54: Precedence doc looks good; confirm code binding for PAGER/ATMOS_COLOR.Matches intended order. Please verify load/bind code actually binds PAGER and ATMOS_COLOR as documented.
internal/exec/describe_component_test.go (1)
18-21: Pager behavior tests LGTM.Good use of gomock to assert pager path vs print path across scenarios.
Also applies to: 117-129, 142-155, 167-173
pkg/config/config_test.go (2)
491-570: parseFlags pager tests LGTM.Covers bare, true/false, named pager, and combo with no-color.
572-627: setLogConfig pager/no-color tests LGTM.Verifies flag plumbing and Color flip when NoColor is set.
- Remove tea.WithMouseCellMotion() to enable mouse text selection in pager - Change pager default from enabled to disabled for better scripting/automation - Add global --pager flag supporting true/false/command values - Add color field to terminal settings (deprecates no_color in config) - Support ATMOS_COLOR and COLOR environment variables - Update documentation with new global-flags.mdx page - Add comprehensive tests for pager and color functionality BREAKING CHANGE: Pager is now disabled by default. Use --pager flag or set pager: true in atmos.yaml to enable. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…uracy - Replace if-else chain with switch statement for better readability (cmd/root.go) - Fix documentation: move Stack/Component flags to Command-Specific section - Remove Breaking Change section from documentation (better suited for release notes) These changes address code quality concerns raised by GitHub's advanced security scanner and improve the accuracy of the documentation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
This commit addresses comprehensive feedback from PR review covering code quality, documentation accuracy, test coverage, and consistency. Changes implemented: • Fix case/whitespace handling in pager value parsing (schema.go) • Enhance --pager documentation with examples and quoting guidance • Fix --no-color flag parsing to properly handle true/false values • Remove redundant per-command pager logic in favor of global handling • Replace direct NoColor field access with IsColorEnabled() method • Fix log level comparison to use canonical constants instead of strings • Add comprehensive test coverage for PAGER env vars and flag precedence Technical improvements: - Pager values now normalized with strings.ToLower(strings.TrimSpace()) - Flag parsing avoids variable shadowing and properly validates values - Removed conflicting per-command pager overrides - Enhanced environment variable precedence testing - Documentation includes practical examples for shell quoting All existing tests pass with improved coverage and robustness. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
This change reduces Atmos output verbosity by changing the default log level from Info to Warning. Also fixes global flags documentation by removing non-global flag sections. - Change default log level from "Info" to "Warning" in config files - Update all test files to use "Warning" instead of "Info" as default - Remove non-global flag sections from global-flags.mdx documentation - Update documentation examples to reflect new default log level 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fix logger level inconsistencies in packer tests (use WarnLevel to match ATMOS_LOGS_LEVEL=Warning) - Add proper resource cleanup with defer blocks to prevent writes to closed pipes - Remove misleading PAGER=cat reference from CI/CD example in global-flags.mdx - Replace with proper ATMOS_PAGER=off for pager disabling - Fix backslash escaping issues in test assertions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add viper import required for TestEnvironmentVariableHandling test function - Resolves undefined viper error after merge conflict resolution 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…tion - Add NO_PAGER environment variable support following standard CLI conventions - Implement proper precedence: CLI flags > NO_PAGER > ATMOS_PAGER > PAGER > config - Add comprehensive test coverage for NO_PAGER functionality (6 new test cases) - Update global-flags.mdx with NO_PAGER documentation and examples - Improve terminal.mdx by removing redundant deprecation warnings - Add clear configuration precedence documentation for pager and color settings - Focus on promoting current best practices rather than deprecated features 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/exec/describe_affected_test.go (2)
23-29: Initial pager config is overwritten; tighten setup. Also fix comment punctuation.
- Line 50 reinitializes d.atmosConfig, wiping the pager=false you set here. Either remove that later assignment or move this initialization to after Line 50 so it takes effect.
- Comments must end with periods per godot. Add a period to the inline comment.
Apply within-range fix:
- Pager: "false", // Initially disabled + Pager: "false", // Initially disabled.And remove the redundant reinitialization later (Line 50).
73-74: End the comment with a period; consider explicit gomock teardown.
- Add a period to satisfy godot.
- Optional: add defer ctrl.Finish() right after creating the controller to ensure expectations are verified.
Within-range punctuation:
- // Enable pager for the tests that expect it to be called + // Enable pager for the tests that expect it to be called.Add explicit teardown (near Line 75):
ctrl := gomock.NewController(t) +defer ctrl.Finish()
📜 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 (2)
internal/exec/describe_affected_test.go(2 hunks)tests/fixtures/scenarios/atmos-describe-affected-with-dependents-and-locked/atmos.yaml(0 hunks)
💤 Files with no reviewable changes (1)
- tests/fixtures/scenarios/atmos-describe-affected-with-dependents-and-locked/atmos.yaml
🧰 Additional context used
📓 Path-based instructions (3)
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Test file naming symmetry: test files mirror implementation structure with _test.go suffix
Unit tests should focus on pure functions and use table-driven tests
ALWAYS use t.Skipf() instead of t.Skip() and provide a clear reason for skipped tests
NEVER use t.Skipf() without a reason
Files:
internal/exec/describe_affected_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods - comments should be complete sentences (enforced by golangci-lint's godot linter)
All errors MUST be wrapped using static errors defined in errors/errors.go; never use dynamic errors directly
Always wrap errors with the static error first and add context-specific details after the static error; use %w verb to preserve error chain
ALWAYS use viper.BindEnv() for environment variable binding, and EVERY env var MUST have an ATMOS_ alternative
Distinguish between logging and UI output: only use logging for system/debug info, and use stderr/UI for prompts, progress, errors, and data for piping should go to stdout
Most text UI MUST go to stderr; only data/results go to stdout for proper piping; use utils.PrintfMessageToTUI() for UI messages
Atmos MUST work on Linux, macOS, and Windows—write portable implementations and prefer SDKs over calling binaries when available
ALWAYS compile after making changes; verify no compilation errors; run tests after successful compile; fix compilation errors immediately before proceeding
Write a test to reproduce the bug before fixing, run the test and confirm failure, fix the bug iteratively, and verify fix doesn't break existing tests
Files:
internal/exec/describe_affected_test.go
internal/exec/**.go
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer extending existing utilities in internal/exec/ and pkg/ over creating new ones—search for existing methods before implementation
Files:
internal/exec/describe_affected_test.go
🧠 Learnings (3)
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
internal/exec/describe_affected_test.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
PR: cloudposse/atmos#1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
internal/exec/describe_affected_test.go
📚 Learning: 2024-12-12T17:13:53.409Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#848
File: pkg/utils/doc_utils.go:19-22
Timestamp: 2024-12-12T17:13:53.409Z
Learning: In `pkg/utils/doc_utils.go`, the `DisplayDocs` function uses the `PAGER` environment variable, which is intentionally user-configurable to allow users to specify custom pager commands that fit their workflow; adding validation to restrict it is not desired.
Applied to files:
internal/exec/describe_affected_test.go
🧬 Code graph analysis (1)
internal/exec/describe_affected_test.go (1)
pkg/schema/schema.go (4)
AtmosConfiguration(26-61)Settings(856-860)AtmosSettings(247-267)Terminal(204-212)
⏰ 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). (8)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
aknysh
added
no-release
…essages This commit fixes test failures on main branch caused by PR #1430 which changed the default pager setting from true to false but didn't update all snapshots. Changes: - Update test snapshots to expect pager: "0" instead of "1" - Remove spurious "Failed to use pager" debug messages from stderr snapshots - Improve pager error messages to include actual error details for debugging The root issue is that test fixtures import from the live main branch's atmos.yaml, causing tests to break when defaults change after merge. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
These changes were released in v1.191.1-test.0. |
Update test snapshots to reflect the corrected pager default value: - Change pager from "1" (true) to "0" (false) in describe configuration output - Remove "Failed to use pager" debug message from stderr (pager is now disabled by default) These changes align with the fix in the previous commit where the pager default was corrected to false as originally intended by PR #1430.
* fix: correct pager default to false as intended by PR #1430 * test: update snapshots for new pager default behavior Update test snapshots to reflect the corrected pager default value: - Change pager from "1" (true) to "0" (false) in describe configuration output - Remove "Failed to use pager" debug message from stderr (pager is now disabled by default) These changes align with the fix in the previous commit where the pager default was corrected to false as originally intended by PR #1430. * docs: add blog post announcing pager default correction Add blog post explaining the regression and its fix: - Describes the history of the pager default issue - Explains impact on users - Provides migration guide for those who relied on pager being enabled - Links to relevant PRs and documentation This blog post helps users understand why the pager behavior may have changed and how to re-enable it if desired. * fix: correct broken link in blog post Change documentation link from /cli/configuration/settings#terminal-settings to /cli/configuration/terminal to fix broken link in blog post. --------- Co-authored-by: Andriy Knysh <[email protected]>
Summary
--pagerflag with smart parsing (true/false/command)Breaking Changes
pager: truetoatmos.yamlor use--pagerflagTest Plan
atmos describe stacks --pagerto verify pager enablesatmos describe stacksto verify pager is disabled by defaultATMOS_PAGER=true atmos describe stacksenables pager--pager=falseexplicitly disables pagergo test ./pkg/pager/... ./pkg/schema/... ./pkg/config/...Changes
pkg/pager/pager.go)--pagerflag to all commands (cmd/root.go)colorfield to terminal settings (pkg/schema/schema.go)ATMOS_COLORandCOLORenvironment variableswebsite/docs/cli/global-flags.mdx)🤖 Generated with Claude Code
Summary by CodeRabbit