Pro status badge on CI PR comments; promote settings.pro to top-level pro - #2883
Open
Erik Osterman (Cloud Posse) (osterman) wants to merge 7 commits into
Open
Pro status badge on CI PR comments; promote settings.pro to top-level pro#2883Erik Osterman (Cloud Posse) (osterman) wants to merge 7 commits into
Erik Osterman (Cloud Posse) (osterman) wants to merge 7 commits into
Conversation
Adds a Pro status badge (green/enabled, silver/disabled) to Native CI's plan/apply/test PR comments, linking to the Atmos Pro dashboard or atmos-pro.com. Promotes settings.pro to a top-level pro: key in both atmos.yaml (CLI connection config) and stack manifests (drift detection/workflow dispatch, now a typed component section alongside vars:/metadata:/settings:), with settings.pro kept as a fully backward compatible deprecated alias (new value wins field-by-field / whole-block when both are set). - pkg/pro/enabled.go: shared ResolveSection/EffectiveEnabledState logic used by pkg/list (upload/counts), pkg/ci (PR badge), consistent with drift dispatch semantics. - pkg/schema, pkg/config, internal/exec/stack_processor_*: new AtmosConfiguration.Pro field + ComponentProSettings typed component section, full stack-processor whitelist plumbing (extraction, merge, overrides, base-component inheritance), manifest/config JSON schema updates, and describe/upload plumbing so pro: survives describe component and describe affected --upload payloads. - Docs and .claude/skills/atmos-pro, atmos-modernization updated to the new top-level pro: form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The complete fixture's vendor.lock.yaml was mutated as a side effect of running the CLI acceptance test suite (a vendor pull test writes to the real fixture lock file); unrelated to the pro: config changes in this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds the changelog post for the CI PR-comment Pro badge and the settings.pro -> top-level pro promotion, and links the shipped milestone into the CI/CD Simplification roadmap initiative. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
August 6, 2026 00:06
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
August 6, 2026 00:06 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
August 6, 2026 00:07 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Resource Changes Found for
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (7)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR moves Atmos Pro configuration to top-level ChangesAtmos Pro configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
pkg/ci/plugins/terraform/plugin_test.go (1)
83-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a table-driven test.
The five cases repeat setup and assertions. Put the component sections and expected
ProEnabledvalue in test cases. Use one subtest loop.As per coding guidelines, use table-driven tests for multiple scenarios.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/ci/plugins/terraform/plugin_test.go` around lines 83 - 154, Refactor TestPlugin_BuildTemplateContext_ProEnabled into a table-driven test with one subtest loop. Define each scenario’s ComponentSection, ComponentSettingsSection, ComponentMetadataSection, and expected ProEnabled value in the test cases, while retaining the shared ConfigAndStacksInfo setup, buildTemplateContext call, error check, type assertion, and assertion behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent-skills/skills/atmos-modernization/SKILL.md`:
- Around line 29-31: Update the migration table entries for CLI connection and
per-component configuration so `settings.pro` and `settings.pro.*` are marked as
deprecated aliases, while the corresponding top-level `pro:` forms are
identified as the preferred replacements. Preserve the existing configuration
details and behavior descriptions.
In `@agent-skills/skills/atmos-pro/SKILL.md`:
- Around line 158-160: Update the atmos.yaml configuration guidance in the
pro/settings.pro section to describe per-field fallback: top-level pro fields
override matching settings.pro fields, while unset top-level fields inherit
their values from settings.pro. Remove the inaccurate claim that the top-level
block wins outright or is never merged.
In `@internal/exec/stack_processor_process_stacks.go`:
- Around line 1375-1387: Update the pro-merging logic in the component
processing flow to deep-merge nested values, preserving stack-global entries
alongside component-local overrides. Replace the manual top-level copy loops
around componentMap[cfg.ProSectionName] with m.Merge(atmosConfig,
[]map[string]any{globalProSection, componentLocalPro}), using the existing local
pro map and retaining the current assignment when the merged result is
non-empty.
In `@pkg/ci/plugins/terraform/template_test.go`:
- Around line 418-502: Add a disabled-Pro test case alongside the existing
test-template scenario in the test matrix, using test command context and a
suitable TestResult. Assert that the rendered output contains
PRO-DISABLED-silver, https://atmos-pro.com, and the disabled footer, while
excluding PRO-ENABLED, the enabled footer, and /dashboard.
In `@pkg/config/load_test.go`:
- Around line 1951-2034: Expand TestLoadConfig_ProSettingsBackwardCompat and
TestResolveProSettings to cover Endpoint, GithubOIDC, MaxPayloadBytes,
GitHubHeadRef, and GitSTS, verifying legacy fallback and top-level pro
precedence per field. Add configuration-loading cases for each Pro field
confirming ATMOS_PRO_* environment values override both pro.* and settings.pro.*
sources, including all relevant loading paths.
In `@tests/snapshots/TestCLICommands_atmos_auth_validate_--verbose.stderr.golden`:
- Around line 2-16: Update the top-level CLI configuration-loading boundary
around resolveProSettings so the settings.pro deprecation warning is emitted at
most once per invocation, even when configuration is resolved repeatedly.
Preserve the warning content and ensure repeated calls do not log duplicates.
In `@website/docs/cli/configuration/settings/pro.mdx`:
- Around line 138-154: Update the `git_config_mode` and `revoke_on_exit`
descriptions under `pro.git_sts` to state that per-integration overrides belong
in the `github/sts` integration `spec`, not in an identity spec. Preserve the
documented global defaults and override behavior while correcting both
references.
---
Nitpick comments:
In `@pkg/ci/plugins/terraform/plugin_test.go`:
- Around line 83-154: Refactor TestPlugin_BuildTemplateContext_ProEnabled into a
table-driven test with one subtest loop. Define each scenario’s
ComponentSection, ComponentSettingsSection, ComponentMetadataSection, and
expected ProEnabled value in the test cases, while retaining the shared
ConfigAndStacksInfo setup, buildTemplateContext call, error check, type
assertion, and assertion behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d6f7a390-30be-429a-961b-7a23b456364c
📒 Files selected for processing (89)
agent-skills/skills/atmos-modernization/SKILL.mdagent-skills/skills/atmos-pro/SKILL.mdcmd/auth/exec.gocmd/mcp/client/messages.gocmd/mcp/client/messages_test.goerrors/errors.gointernal/exec/describe_affected_test.gointernal/exec/describe_affected_upload.gointernal/exec/describe_affected_utils_2.gointernal/exec/describe_component.gointernal/exec/describe_dependents.gointernal/exec/stack_processor_cache.gointernal/exec/stack_processor_merge.gointernal/exec/stack_processor_process_stacks.gointernal/exec/stack_processor_process_stacks_helpers.gointernal/exec/stack_processor_process_stacks_helpers_extraction.gointernal/exec/stack_processor_process_stacks_helpers_inheritance.gointernal/exec/stack_processor_process_stacks_helpers_overrides.gointernal/exec/stack_processor_utils.gopkg/ci/plugins/terraform/context.gopkg/ci/plugins/terraform/plugin.gopkg/ci/plugins/terraform/plugin_test.gopkg/ci/plugins/terraform/template_test.gopkg/ci/plugins/terraform/templates/apply.mdpkg/ci/plugins/terraform/templates/plan.mdpkg/ci/plugins/terraform/templates/test.mdpkg/ci/plugins/terraform/testdata/golden/apply_failure.mdpkg/ci/plugins/terraform/testdata/golden/apply_success.mdpkg/ci/plugins/terraform/testdata/golden/apply_with_outputs.mdpkg/ci/plugins/terraform/testdata/golden/plan_creates_only.mdpkg/ci/plugins/terraform/testdata/golden/plan_destroys_warning.mdpkg/ci/plugins/terraform/testdata/golden/plan_failure.mdpkg/ci/plugins/terraform/testdata/golden/plan_no_changes.mdpkg/ci/plugins/terraform/testdata/golden/plan_no_changes_with_changed_result.mdpkg/ci/plugins/terraform/testdata/golden/plan_with_warnings.mdpkg/config/const.gopkg/config/load.gopkg/config/load_config_args.gopkg/config/load_test.gopkg/datafetcher/schema/atmos/config/1.0.jsonpkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/datafetcher/schema_section_coverage_test.gopkg/list/list_instances.gopkg/list/list_instances_cmd_test.gopkg/list/list_instances_pro_test.gopkg/pro/api_client.gopkg/pro/api_client_test.gopkg/pro/commit.gopkg/pro/commit_test.gopkg/pro/enabled.gopkg/pro/enabled_test.gopkg/pro/interface_test.gopkg/schema/instance.gopkg/schema/pro.gopkg/schema/schema.gopkg/telemetry/utils.gotests/fixtures/schemas/atmos/atmos-manifest/1.0/atmos-manifest.jsontests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stdout.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Environment_Variable.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Environment_Variable.stdout.goldentests/snapshots/TestCLICommands_Valid_log_file_in_env_should_be_priortized_over_config.stdout.goldentests/snapshots/TestCLICommands_Valid_log_file_in_flag_should_be_priortized_over_env_and_config.stdout.goldentests/snapshots/TestCLICommands_Valid_log_level_in_env_should_be_priortized_over_config.stderr.goldentests/snapshots/TestCLICommands_Valid_log_level_in_env_should_be_priortized_over_config.stdout.goldentests/snapshots/TestCLICommands_Valid_log_level_in_flag_should_be_priortized_over_env_and_config.stderr.goldentests/snapshots/TestCLICommands_Valid_log_level_in_flag_should_be_priortized_over_env_and_config.stdout.goldentests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.goldentests/snapshots/TestCLICommands_atmos_auth_validate_--verbose.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.goldentests/snapshots/TestCLICommands_atmos_vendor_pull_component_using_SSH.stderr.goldentests/snapshots/TestCLICommands_atmos_vendor_pull_using_SSH.stderr.goldentests/snapshots/TestCLICommands_atmos_vendor_pull_with_custom_detector_and_handling_credentials_leakage.stderr.goldentests/snapshots/TestCLICommands_echo_info_runs_with_verbose_flag.stderr.goldentests/snapshots/TestCLICommands_indentation.stdout.goldentests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.goldenwebsite/blog/2026-08-05-pro-badge-and-top-level-config.mdxwebsite/docs/cli/commands/list/list-instances.mdxwebsite/docs/cli/configuration/auth/index.mdxwebsite/docs/cli/configuration/settings/pro.mdxwebsite/docs/mcp/mcp.mdxwebsite/docs/pro/pro.mdxwebsite/docs/stacks/components/index.mdxwebsite/src/data/roadmap.js
💤 Files with no reviewable changes (4)
- tests/snapshots/TestCLICommands_Valid_log_level_in_flag_should_be_priortized_over_env_and_config.stdout.golden
- tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stdout.golden
- tests/snapshots/TestCLICommands_Valid_Log_Level_in_Environment_Variable.stdout.golden
- tests/snapshots/TestCLICommands_Valid_log_level_in_env_should_be_priortized_over_config.stdout.golden
Comment on lines
+29
to
+31
| | `cloudposse/github-action-atmos-terraform-drift-*` | `pro.drift_detection` plus `atmos terraform plan --upload-status` | | ||
| | `settings.pro` in `atmos.yaml` (CLI connection config: `base_url`/`token`/`workspace_id`/etc.) | Top-level `pro:` in `atmos.yaml` (deprecated alias, still works) | | ||
| | `settings.pro.*` per component/stack (`enabled`/`drift_detection`/`pull_request`/`release`/`merge_group`) | Top-level `pro:` component section, a sibling of `vars:`/`metadata:`/`settings:` (deprecated alias, still works) | |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Attach the deprecation label to settings.pro.
The table currently labels the new top-level pro: forms as deprecated. settings.pro is the deprecated alias. Mark top-level pro: as the preferred replacement.
Proposed wording
-| `settings.pro` in `atmos.yaml` | Top-level `pro:` in `atmos.yaml` (deprecated alias, still works) |
-| `settings.pro.*` per component/stack | Top-level `pro:` component section (deprecated alias, still works) |
+| `settings.pro` in `atmos.yaml` | Top-level `pro:` in `atmos.yaml` (`settings.pro` remains supported as a deprecated alias) |
+| `settings.pro.*` per component/stack | Top-level `pro:` component section (`settings.pro` remains supported as a deprecated alias) |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent-skills/skills/atmos-modernization/SKILL.md` around lines 29 - 31,
Update the migration table entries for CLI connection and per-component
configuration so `settings.pro` and `settings.pro.*` are marked as deprecated
aliases, while the corresponding top-level `pro:` forms are identified as the
preferred replacements. Preserve the existing configuration details and behavior
descriptions.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (70.40%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2883 +/- ##
==========================================
- Coverage 82.76% 82.75% -0.02%
==========================================
Files 1861 1862 +1
Lines 180438 180650 +212
==========================================
+ Hits 149342 149497 +155
- Misses 23309 23339 +30
- Partials 7787 7814 +27
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Deep-merge pro: for custom component types (container/emulator), matching the built-in types' m.Merge pattern instead of a shallow top-level-key copy that let a component-local pro.pull_request with one activity wipe out other globally-configured activities under the same key. - Dedupe the 'settings.pro is deprecated' debug notice to once per process instead of once per LoadConfig call (which fires repeatedly per CLI invocation), regenerating the golden snapshots that captured the noise. - Fix skill/doc wording: attach "deprecated alias" to settings.pro instead of the new pro: form; correct git_sts.git_config_mode/revoke_on_exit override location (github/sts integration spec, not an identity spec); split the atmos-pro skill's precedence note into its two real behaviors (per-field fallback for atmos.yaml, whole-block precedence for stack/component pro:). - Expand pro.go migration test coverage to every ProSettings field and add an ATMOS_PRO_* env-var precedence test; add a disabled-Pro test case for the CI "test" template scenario. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
August 6, 2026 01:44 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/exec/stack_processor_process_stacks.go (1)
945-956: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep Helm in the built-in component set.
ProcessStackConfigprocesses Helm before the custom-component loop.builtInTypesomitscfg.HelmComponentType, so that loop overwritesallComponents["helm"].The replacement path merges only
globalProSection. It discardsglobalAndHelmPro, so settings underhelm.prodo not reach Helm components. Add Helm tobuiltInTypesand add a Helm-specific regression test.Proposed fix
builtInTypes := map[string]bool{ cfg.TerraformComponentType: true, cfg.HelmfileComponentType: true, cfg.PackerComponentType: true, cfg.AnsibleComponentType: true, cfg.KubernetesComponentType: true, + cfg.HelmComponentType: true, }Also applies to: 1281-1281
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/exec/stack_processor_process_stacks.go` around lines 945 - 956, Update ProcessStackConfig and its built-in component classification to include cfg.HelmComponentType in builtInTypes, preventing the custom-component loop from overwriting the processed Helm entry. Ensure the Helm replacement path uses globalAndHelmPro rather than only globalProSection so helm.pro settings reach Helm components, and add a regression test covering both behaviors.agent-skills/skills/atmos-pro/SKILL.md (1)
17-23: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the precedence summary consistent.
Lines 20-21 say that a top-level
pro:block always wins. Lines 158-161 define per-field fallback foratmos.yaml. These statements conflict. Document per-field fallback foratmos.yaml, and reserve whole-block precedence for component/stack configuration.Suggested wording
- `settings.pro` is a **deprecated alias** for both — Atmos still reads it, but an explicit top-level `pro:` block always wins over `settings.pro:` when both are set. + `settings.pro` is a **deprecated alias** for both — Atmos still reads it. In `atmos.yaml`, each top-level `pro.<field>` overrides the matching `settings.pro.<field>`, while unset fields fall back to `settings.pro`. For component/stack configuration, an explicit top-level `pro:` block takes precedence as a whole.Also applies to: 158-165
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent-skills/skills/atmos-pro/SKILL.md` around lines 17 - 23, Update the precedence guidance in the top-level CLI configuration section and the per-field fallback description near the per-component/stack configuration section. For atmos.yaml, document that explicitly set fields in the top-level pro: block override corresponding settings.pro fields while unspecified fields fall back to settings.pro; reserve whole-block precedence, where the top-level pro: block replaces settings.pro entirely, for component/stack configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@agent-skills/skills/atmos-pro/SKILL.md`:
- Around line 17-23: Update the precedence guidance in the top-level CLI
configuration section and the per-field fallback description near the
per-component/stack configuration section. For atmos.yaml, document that
explicitly set fields in the top-level pro: block override corresponding
settings.pro fields while unspecified fields fall back to settings.pro; reserve
whole-block precedence, where the top-level pro: block replaces settings.pro
entirely, for component/stack configuration.
In `@internal/exec/stack_processor_process_stacks.go`:
- Around line 945-956: Update ProcessStackConfig and its built-in component
classification to include cfg.HelmComponentType in builtInTypes, preventing the
custom-component loop from overwriting the processed Helm entry. Ensure the Helm
replacement path uses globalAndHelmPro rather than only globalProSection so
helm.pro settings reach Helm components, and add a regression test covering both
behaviors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c77820f0-fa43-42ea-8552-eab1d95967e0
📒 Files selected for processing (21)
agent-skills/skills/atmos-modernization/SKILL.mdagent-skills/skills/atmos-pro/SKILL.mdinternal/exec/stack_processor_process_stacks.gointernal/exec/stack_processor_process_stacks_test.gopkg/ci/plugins/terraform/template_test.gopkg/config/load.gopkg/config/load_test.gotests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Environment_Variable.stderr.goldentests/snapshots/TestCLICommands_Valid_log_file_in_env_should_be_priortized_over_config.stdout.goldentests/snapshots/TestCLICommands_Valid_log_file_in_flag_should_be_priortized_over_env_and_config.stdout.goldentests/snapshots/TestCLICommands_Valid_log_level_in_env_should_be_priortized_over_config.stderr.goldentests/snapshots/TestCLICommands_Valid_log_level_in_flag_should_be_priortized_over_env_and_config.stderr.goldentests/snapshots/TestCLICommands_atmos_auth_validate_--verbose.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_vendor_pull_component_using_SSH.stderr.goldentests/snapshots/TestCLICommands_atmos_vendor_pull_using_SSH.stderr.goldentests/snapshots/TestCLICommands_atmos_vendor_pull_with_custom_detector_and_handling_credentials_leakage.stderr.goldentests/snapshots/TestCLICommands_echo_info_runs_with_verbose_flag.stderr.goldenwebsite/docs/cli/configuration/settings/pro.mdx
💤 Files with no reviewable changes (13)
- tests/snapshots/TestCLICommands_Valid_log_level_in_flag_should_be_priortized_over_env_and_config.stderr.golden
- tests/snapshots/TestCLICommands_Valid_Log_Level_in_Environment_Variable.stderr.golden
- tests/snapshots/TestCLICommands_atmos_vendor_pull_using_SSH.stderr.golden
- tests/snapshots/TestCLICommands_atmos_vendor_pull_component_using_SSH.stderr.golden
- tests/snapshots/TestCLICommands_Valid_log_file_in_env_should_be_priortized_over_config.stdout.golden
- tests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden
- tests/snapshots/TestCLICommands_echo_info_runs_with_verbose_flag.stderr.golden
- tests/snapshots/TestCLICommands_Valid_log_level_in_env_should_be_priortized_over_config.stderr.golden
- tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
- tests/snapshots/TestCLICommands_atmos_auth_validate_--verbose.stderr.golden
- tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.golden
- tests/snapshots/TestCLICommands_atmos_vendor_pull_with_custom_detector_and_handling_credentials_leakage.stderr.golden
- tests/snapshots/TestCLICommands_Valid_log_file_in_flag_should_be_priortized_over_env_and_config.stdout.golden
🚧 Files skipped from review as they are similar to previous changes (3)
- agent-skills/skills/atmos-modernization/SKILL.md
- pkg/config/load_test.go
- pkg/ci/plugins/terraform/template_test.go
…85%+ Adds tests for the coverage gaps Codecov flagged on this PR's patch (pkg/config, internal/exec stack processing, pkg/list, pkg/schema), plus two accompanying production fixes found while writing them: load.go's pro.base_url/endpoint defaults no longer get treated as user-authored legacy config, and a malformed/typo'd component-level `pro:` section now warns instead of aborting processing of sibling components (matching how pro.ResolveSection already tolerates it downstream). Diff coverage is now ~89% (up from 66.39%). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
August 6, 2026 13:56 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/schema/pro_decode_test.go (1)
11-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the decoder coverage table-driven and complete.
TestDecodeComponentPro_FullShapedoes not coverReleaseorMergeGroup, although both are supported byComponentProSettings.Use table-driven cases for the decode scenarios. Add populated input and assertions for both missing dispatch fields.
As per coding guidelines, new feature tests must be comprehensive and table-driven.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/schema/pro_decode_test.go` around lines 11 - 80, Refactor the DecodeComponentPro tests into table-driven cases covering nil, empty, invalid, partial, unknown-field, and full configurations. Expand the full-shape input and assertions in TestDecodeComponentPro_FullShape’s replacement to populate and verify both supported Release and MergeGroup settings, including their dispatch fields, while preserving existing coverage for Enabled, DriftDetection, and PullRequest.Source: Coding guidelines
pkg/schema/pro_decode.go (1)
10-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one error identity for invalid component
proconfiguration.
errors/errors.go:479already definesErrInvalidComponentPro. Reusing it keepserrors.Isconsistent across schema decoding and stack processing.
pkg/schema/pro_decode.go#L10-L13: removeErrInvalidComponentProConfigand wrapErrInvalidComponentProafter verifying no import cycle exists.pkg/schema/pro_decode_test.go#L23-L28: asserterrors.Is(err, errUtils.ErrInvalidComponentPro).pkg/schema/pro_decode_test.go#L62-L66: asserterrors.Is(err, errUtils.ErrInvalidComponentPro).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/schema/pro_decode.go` around lines 10 - 13, Use the existing errUtils.ErrInvalidComponentPro identity throughout component pro decoding: remove ErrInvalidComponentProConfig from pkg/schema/pro_decode.go and wrap errUtils.ErrInvalidComponentPro after confirming no import cycle. Update pkg/schema/pro_decode_test.go lines 23-28 and 62-66 to assert errors.Is(err, errUtils.ErrInvalidComponentPro).Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/exec/stack_processor_process_stacks_helpers_extraction.go`:
- Around line 61-76: Update the ComponentPro handling in the stack-processing
helper so result.ComponentPro is assigned only when schema.DecodeComponentPro
succeeds; leave it unset when validation rejects an unrecognized field such as
“enable”. Keep the validation warning non-fatal so sibling components continue
processing, and add a regression test covering this typo and confirming Pro is
not enabled from the invalid map.
---
Nitpick comments:
In `@pkg/schema/pro_decode_test.go`:
- Around line 11-80: Refactor the DecodeComponentPro tests into table-driven
cases covering nil, empty, invalid, partial, unknown-field, and full
configurations. Expand the full-shape input and assertions in
TestDecodeComponentPro_FullShape’s replacement to populate and verify both
supported Release and MergeGroup settings, including their dispatch fields,
while preserving existing coverage for Enabled, DriftDetection, and PullRequest.
In `@pkg/schema/pro_decode.go`:
- Around line 10-13: Use the existing errUtils.ErrInvalidComponentPro identity
throughout component pro decoding: remove ErrInvalidComponentProConfig from
pkg/schema/pro_decode.go and wrap errUtils.ErrInvalidComponentPro after
confirming no import cycle. Update pkg/schema/pro_decode_test.go lines 23-28 and
62-66 to assert errors.Is(err, errUtils.ErrInvalidComponentPro).
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: dc2ded76-d8b6-4bf6-8c85-34bf3798f38c
📒 Files selected for processing (14)
internal/exec/describe_affected_utils_2_test.gointernal/exec/describe_dependents_test.gointernal/exec/stack_processor_process_stacks_helpers_extraction.gointernal/exec/stack_processor_process_stacks_helpers_test.gointernal/exec/stack_processor_process_stacks_test.gointernal/exec/stack_processor_utils_test.gopkg/config/load.gopkg/config/load_test.gopkg/config/testdata/default-config-snapshot.yamlpkg/list/list_instances_comprehensive_test.gopkg/list/list_instances_upload_test.gopkg/schema/pro_decode.gopkg/schema/pro_decode_test.gopkg/telemetry/utils_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/config/load.go
…ro golden snapshots CI's Acceptance Tests job was failing on two fronts: - TestLoadConfig_ProDefaultsDoNotLeakIntoLegacySettings compared Settings.Pro against a zero value, but GitHub Actions sets GITHUB_HEAD_REF on every pull_request run, which pkg/config binds into Settings.Pro.GitHubHeadRef -- passing locally (unset) and failing only in CI. tests/cli_test.go's TestMain already unsets other CI-ambient vars (GITHUB_ACTIONS, ATMOS_CHDIR) for the same reason; GITHUB_HEAD_REF joins that list, and the pkg/config unit test isolates it directly via t.Setenv. - 20 CLI golden snapshots still expected settings.pro.base_url/endpoint to be populated and the 'settings.pro' deprecation notice to fire unconditionally -- both stale from before this branch's earlier fix that moved pro.base_url/pro.endpoint defaults off the deprecated settings.pro path. Regenerated via -regenerate-snapshots; every change is a pure deletion of the now-obsolete lines. Also confirmed pkg/scheduler/adapters' TestExecuteTerraformConcurrentHooksUseNodeWriters failure in the same CI run is an unrelated pre-existing flake (passes 5/5 locally; no commit on this branch touches pkg/scheduler) -- left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CodeRabbit review (PR #2883, discussion_r3729413928): extractComponentSections assigned result.ComponentPro before schema.DecodeComponentPro validated it, so a typo'd pro: {enable: false} (meant "enabled") still flowed downstream as the raw map. pro.ResolveSection ignores unrecognized keys, so the typo silently resolved to Pro's default-enabled behavior -- the opposite of what the user wrote. Now the raw map is only assigned after decoding succeeds; the warning stays non-fatal so sibling components still process. Updated TestProcessStackConfig_ProSectionUnknownKeyDoesNotBlockProcessing to assert the pro: section is absent (not silently enabling Pro) rather than passed through unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
plan/apply/testPR comments now show a Pro status badge in the same row as the other result badges: greenPRO-ENABLED(linking to the Atmos Pro dashboard) or silverPRO-DISABLED(linking to atmos-pro.com), plus a footer line stating whether Atmos Pro is enabled for that component.settings.prois promoted to a top-levelpro:key inatmos.yaml(CLI connection config:base_url/token/workspace_id/github_oidc/git_sts/etc., a sibling ofauth:/docs:/ci:), and to a top-levelpro:component section in stack manifests (drift detection + GitHub-event workflow dispatch, a sibling ofvars:/metadata:/settings:) — the first typed (schema-validated) component section in the codebase.settings.prokeeps working everywhere as a deprecated alias: each field falls back independently inatmos.yaml, and an explicit top-levelpro:block takes whole-block precedence oversettings.pro:at the component level. A deprecation notice is logged whenever the legacy path is read.pro:component section: extraction, deep-merge (global/type-default/base-component/component/overrides), base-component (metadata.component) inheritance,describe componentanddescribe affected --upload/list instances --uploadpayloads, and both the CLI-config and stack-manifest JSON schemas.describe component's output filter was silently dropping the newpro:section, anddescribe affected --upload/dependents payloads weren't carrying it forward to Atmos Pro.cli/configuration/settings/pro,pro/pro,auth,mcp,list-instances,stacks/components) and theatmos-pro/atmos-modernizationClaude skills to the new top-levelpro:form.why
atmos prois a top-level CLI command group, but its configuration was nested three levels deep undersettings.proin two unrelated places (global CLI connection config vs. per-component drift/workflow-dispatch config) that only coincidentally shared a name — hard to discover, hard to search for, and invisible in CI output.references
Summary by CodeRabbit
New Features
pro:configuration for Atmos Pro settings across configuration files and stack manifests.Bug Fixes
Documentation
Compatibility
settings.proas a deprecated alias with precedence and fallback support.