Repository navigation
feat: add atmos pro install command with workflow scaffolding - #2288
Open
Erik Osterman (Cloud Posse) (osterman) wants to merge 27 commits into
Open
Erik Osterman (Cloud Posse) (osterman) wants to merge 27 commits into
Erik Osterman (Cloud Posse) (osterman) wants to merge 27 commits into
Conversation
…vider Modernize the `pro` command from the legacy direct-registration pattern to the CommandProvider registry pattern, and add a new `atmos pro install` subcommand that scaffolds GitHub Actions workflows, auth profiles, and stack configuration for Atmos Pro integration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
April 6, 2026 21:22
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new cohesive Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as "atmos pro install"
participant Config as "Resolve Paths"
participant Prompt as "User Prompt"
participant Installer as "Installer"
participant FS as "FileWriter (OS/Mock)"
participant Browser as "Browser (open URL)"
User->>CLI: run command (flags)
CLI->>Config: resolve install paths (global flags / config)
alt not --yes and not --dry-run
CLI->>Prompt: confirm installation?
Prompt-->>CLI: confirm/decline
end
CLI->>Installer: create installer (BasePath, StacksBasePath, Force, OnConflict)
alt --dry-run
Installer->>FS: check file existence (no writes)
FS-->>Installer: existence info
Installer-->>CLI: dry-run report
else
Installer->>FS: check file existence
alt conflict and not --force
CLI->>Prompt: "Overwrite <path>?"
Prompt-->>Installer: decision
end
Installer->>FS: mkdir/write files
FS-->>Installer: write results
Installer-->>CLI: install result (created/updated/skipped)
alt not --yes
CLI->>Prompt: "Open onboarding workspace?"
Prompt-->>CLI: decision
alt open chosen
CLI->>Browser: open workspace URL
Browser-->>CLI: success/fail
end
end
end
CLI->>User: print report & next steps
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
cmd/pro/lock.go (1)
10-16: Add concrete command examples to help output.The command definition is clear, but it lacks
Exampleusage, which makes first-time usage harder.💡 Suggested improvement
var lockCmd = &cobra.Command{ Use: "lock", Short: "Lock a stack", Long: `This command calls the atmos pro API and locks a stack`, + Example: ` $ atmos pro lock -c vpc -s plat-ue2-dev + $ atmos pro lock -c vpc -s plat-ue2-dev -m "maintenance window" -t 1800`, Args: cobra.NoArgs, RunE: e.ExecuteProLockCommand, }As per coding guidelines: "Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/pro/lock.go` around lines 10 - 16, Add concrete usage examples to the Cobra command definition for lockCmd: populate the Example field on lockCmd with one or more realistic command lines (e.g., basic usage and a usage with common flags/env) so users see how to call the command; update the lockCmd declaration (the variable named lockCmd that uses e.ExecuteProLockCommand) to include Example: "<example strings>" and ensure examples follow Cobra/Go doc conventions and reflect typical invocations of the pro lock command.cmd/pro/unlock.go (1)
10-16: Add examples topro unlockhelp.The command is missing
Exampleguidance for common usage.💡 Suggested improvement
var unlockCmd = &cobra.Command{ Use: "unlock", Short: "Unlock a stack", Long: `This command calls the atmos pro API and unlocks a stack`, + Example: ` $ atmos pro unlock -c vpc -s plat-ue2-dev`, Args: cobra.NoArgs, RunE: e.ExecuteProUnlockCommand, }As per coding guidelines: "Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/pro/unlock.go` around lines 10 - 16, The unlockCmd Cobra command is missing an Example field in its help; add an Example string to the unlockCmd definition (near the existing Use/Short/Long/Args/RunE) showing common usage patterns (e.g., basic unlock, unlocking with flags or environment examples) so users see concrete commands in help output; update the unlockCmd variable's struct to include Example: with one or more lines illustrating typical invocations that correspond to the behavior implemented by ExecuteProUnlockCommand.pkg/pro/install/templates/atmos-pro-terraform-drift-detection.yaml (1)
11-36: Add overlap and hang protection for scheduled drift detection.Line 11 schedule can stack runs under load. Add workflow/job guards to keep one active run and cap execution time.
♻️ Suggested hardening
on: schedule: - cron: "0 */4 * * *" workflow_dispatch: {} +concurrency: + group: atmos-pro-drift-detection + cancel-in-progress: false + permissions: id-token: write contents: read issues: write jobs: detect: + timeout-minutes: 60 runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/pro/install/templates/atmos-pro-terraform-drift-detection.yaml` around lines 11 - 36, Add concurrency and job timeout to prevent overlapping and hanging runs: add a top-level concurrency stanza with a unique group (e.g., using vars.ATMOS_VERSION or 'atmos-drift-detect') and cancel-in-progress: true to keep only one active workflow, and add timeout-minutes under the detect job (or use a step-level timeout wrapper) to cap execution time for the job that runs the "Detect Drift" step which executes atmos pro drift detect; update the file to include these keys around the existing schedule and the detect job to ensure overlap and hang protection.pkg/pro/install/templates/atmos-pro-terraform-drift-remediation.yaml (1)
11-37: Prevent duplicate remediation runs per issue.Line 11 onward allows multiple runs for the same issue when labels are toggled/reapplied. Add a concurrency key on issue number to avoid overlapping remediations.
♻️ Suggested hardening
on: issues: types: - labeled +concurrency: + group: atmos-pro-drift-remediation-${{ github.event.issue.number }} + cancel-in-progress: true + permissions: id-token: write contents: read issues: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/pro/install/templates/atmos-pro-terraform-drift-remediation.yaml` around lines 11 - 37, Add a GitHub Actions concurrency block to the remediate job to ensure only one remediation runs per issue at a time: inside the job named "remediate" add a concurrency stanza with a group key that includes the issue number (e.g., group: remediate-issue-${{ github.event.issue.number }}) and set cancel-in-progress to false so new triggers for the same issue are queued and do not overlap with the running run.pkg/pro/install/install.go (1)
276-299: Edge case: prepending import to files with YAML document markers.If
_defaults.yamlstarts with---(YAML document marker) or comments, prependingimport:creates a multi-document file or puts the import before the marker. This is an edge case but worth noting.Possible enhancement
// No import section found - prepend one. +// Handle files starting with document marker. +if strings.HasPrefix(strings.TrimSpace(content), "---") { + // Insert after the document marker line + for idx, line := range lines { + if strings.TrimSpace(line) == "---" { + result := make([]string, 0, len(lines)+2) + result = append(result, lines[:idx+1]...) + result = append(result, "import:") + result = append(result, importLine) + result = append(result, lines[idx+1:]...) + return strings.Join(result, "\n") + } + } +} importSection := "import:\n" + importLine + "\n\n" return importSection + content🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/pro/install/install.go` around lines 276 - 299, The addImport function incorrectly prepends an import section before leading YAML document markers or comments; update addImport to detect and preserve a leading YAML document marker ("---") and any consecutive leading comment lines (lines starting with "#") by inserting the new "import:" section after those markers/comments instead of at the file start; modify the logic in addImport (use the existing lines slice and importLine/importSection names) to scan from the top for a contiguous block of empty lines, a single initial "---" and subsequent comment lines, compute the insertion index after that block, and insert the import section there when no existing "import:" header is found.cmd/pro/install.go (1)
54-96: Addperf.TracktorunInstall.Per coding guidelines, public functions with I/O should include performance tracking. The
runInstallfunction performs file operations and user prompts.Suggested fix
func runInstall(cmd *cobra.Command, _ []string) error { + defer perf.Track(nil, "pro.runInstall")() + yes, _ := cmd.Flags().GetBool("yes")Also add the import:
+ "github.com/cloudposse/atmos/pkg/perf"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/pro/install.go` around lines 54 - 96, Add a perf.Track call to runInstall to measure its I/O work: invoke perf.Track at the start of runInstall (and defer its finish) so the whole function runtime including resolveInstallPaths, flags.PromptForConfirmation, installer.DryRun and installer.Install is recorded; also add the perf import to the file. Ensure the tracking uses a clear name (e.g., "cmd.pro.install.runInstall") and that the defer is placed immediately after starting the track so all subsequent operations are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/pro/lock.go`:
- Around line 21-22: The help text for the flags registered on lockCmd is
inconsistent with their actual default values; update the flag registrations so
the declared defaults match runtime defaults by setting the StringP for
"message" to default "Locked by Atmos" and the Int32P for "ttl" to default 30
(keep the help text as-is), i.e., change the default values passed to
lockCmd.PersistentFlags().StringP("message", "m", ...) and
lockCmd.PersistentFlags().Int32P("ttl", "t", ...).
In `@cmd/pro/unlock.go`:
- Around line 19-20: The flag descriptions for the unlock command are incorrect:
update the two calls to unlockCmd.PersistentFlags().StringP("component", "c",
...) and unlockCmd.PersistentFlags().StringP("stack", "s", ...) to say "Specify
the Atmos component to unlock" and "Specify the Atmos stack to unlock"
respectively so the help text accurately reflects the command action.
In `@pkg/pro/install/install.go`:
- Around line 140-146: The current block treats any ReadFile error as a silent
skip; update the logic around i.writer.FileExists/defaultsPath and
i.writer.ReadFile so that when ReadFile returns an error you capture and report
it instead of silently appending to result.SkippedFiles: keep the successful
path (if err == nil && !hasImport(...)) that appends to result.UpdatedFiles, but
in the else branch when err != nil log or record the error (e.g. append a
formatted error to result.Errors or add an entry to a result.FileErrors map
keyed by i.defaultsRelPath()) using the actual err from i.writer.ReadFile and
then mark the file appropriately (SkippedFiles or FailedFiles); reference the
symbols defaultsPath, i.writer.ReadFile, hasImport, i.defaultsRelPath,
result.UpdatedFiles and result.SkippedFiles when making the change.
In `@pkg/pro/install/templates/atmos-pro-terraform-apply.yaml`:
- Line 42: The inline shell command `atmos terraform apply ${{ inputs.component
}} -s ${{ inputs.stack }} --auto-approve` interpolates unquoted workflow inputs
and is vulnerable to command injection and breaking on spaces; fix it by quoting
each interpolation (e.g., wrap `${{ inputs.component }}` and `${{ inputs.stack
}}` in quotes) so the shell treats them as single arguments, or pass them as
action inputs/arguments safely rather than concatenating into an unquoted bash
line; locate the command string and update the `${{ inputs.component }}` and
`${{ inputs.stack }}` usages to be quoted before executing.
In `@pkg/pro/install/templates/atmos-pro-terraform-plan.yaml`:
- Line 42: The workflow invokes the shell with unquoted inputs in the command
"atmos terraform plan ${{ inputs.component }} -s ${{ inputs.stack }}", which is
unsafe for values with spaces or special chars; update that invocation to wrap
both inputs in quotes so the shell sees each input as a single argument (i.e.,
quote the expressions for inputs.component and inputs.stack in the atmos
terraform plan command) and ensure any downstream usage still expects a single
argument per input.
---
Nitpick comments:
In `@cmd/pro/install.go`:
- Around line 54-96: Add a perf.Track call to runInstall to measure its I/O
work: invoke perf.Track at the start of runInstall (and defer its finish) so the
whole function runtime including resolveInstallPaths,
flags.PromptForConfirmation, installer.DryRun and installer.Install is recorded;
also add the perf import to the file. Ensure the tracking uses a clear name
(e.g., "cmd.pro.install.runInstall") and that the defer is placed immediately
after starting the track so all subsequent operations are covered.
In `@cmd/pro/lock.go`:
- Around line 10-16: Add concrete usage examples to the Cobra command definition
for lockCmd: populate the Example field on lockCmd with one or more realistic
command lines (e.g., basic usage and a usage with common flags/env) so users see
how to call the command; update the lockCmd declaration (the variable named
lockCmd that uses e.ExecuteProLockCommand) to include Example: "<example
strings>" and ensure examples follow Cobra/Go doc conventions and reflect
typical invocations of the pro lock command.
In `@cmd/pro/unlock.go`:
- Around line 10-16: The unlockCmd Cobra command is missing an Example field in
its help; add an Example string to the unlockCmd definition (near the existing
Use/Short/Long/Args/RunE) showing common usage patterns (e.g., basic unlock,
unlocking with flags or environment examples) so users see concrete commands in
help output; update the unlockCmd variable's struct to include Example: with one
or more lines illustrating typical invocations that correspond to the behavior
implemented by ExecuteProUnlockCommand.
In `@pkg/pro/install/install.go`:
- Around line 276-299: The addImport function incorrectly prepends an import
section before leading YAML document markers or comments; update addImport to
detect and preserve a leading YAML document marker ("---") and any consecutive
leading comment lines (lines starting with "#") by inserting the new "import:"
section after those markers/comments instead of at the file start; modify the
logic in addImport (use the existing lines slice and importLine/importSection
names) to scan from the top for a contiguous block of empty lines, a single
initial "---" and subsequent comment lines, compute the insertion index after
that block, and insert the import section there when no existing "import:"
header is found.
In `@pkg/pro/install/templates/atmos-pro-terraform-drift-detection.yaml`:
- Around line 11-36: Add concurrency and job timeout to prevent overlapping and
hanging runs: add a top-level concurrency stanza with a unique group (e.g.,
using vars.ATMOS_VERSION or 'atmos-drift-detect') and cancel-in-progress: true
to keep only one active workflow, and add timeout-minutes under the detect job
(or use a step-level timeout wrapper) to cap execution time for the job that
runs the "Detect Drift" step which executes atmos pro drift detect; update the
file to include these keys around the existing schedule and the detect job to
ensure overlap and hang protection.
In `@pkg/pro/install/templates/atmos-pro-terraform-drift-remediation.yaml`:
- Around line 11-37: Add a GitHub Actions concurrency block to the remediate job
to ensure only one remediation runs per issue at a time: inside the job named
"remediate" add a concurrency stanza with a group key that includes the issue
number (e.g., group: remediate-issue-${{ github.event.issue.number }}) and set
cancel-in-progress to false so new triggers for the same issue are queued and do
not overlap with the running run.
🪄 Autofix (Beta)
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
Run ID: e65ba3fd-a7da-434a-acba-2db874ea01e6
📒 Files selected for processing (25)
cmd/pro.gocmd/pro/install.gocmd/pro/lock.gocmd/pro/markdown/atmos_pro_install.mdcmd/pro/markdown/atmos_pro_install_next_steps.mdcmd/pro/pro.gocmd/pro/pro_test.gocmd/pro/unlock.gocmd/pro_lock.gocmd/pro_lock_test.gocmd/pro_test.gocmd/pro_unlock.gocmd/pro_unlock_test.gocmd/root.gopkg/pro/install/install.gopkg/pro/install/install_test.gopkg/pro/install/options.gopkg/pro/install/templates.gopkg/pro/install/templates/atmos-pro-mixin.yamlpkg/pro/install/templates/atmos-pro-terraform-apply.yamlpkg/pro/install/templates/atmos-pro-terraform-drift-detection.yamlpkg/pro/install/templates/atmos-pro-terraform-drift-remediation.yamlpkg/pro/install/templates/atmos-pro-terraform-plan.yamlpkg/pro/install/templates/defaults-snippet.yamlpkg/pro/install/templates/github-profile-atmos.yaml
💤 Files with no reviewable changes (6)
- cmd/pro_test.go
- cmd/pro.go
- cmd/pro_unlock_test.go
- cmd/pro_unlock.go
- cmd/pro_lock_test.go
- cmd/pro_lock.go
Align scaffolded templates with a production Atmos Pro deployment: - Delete hallucinated drift-remediation workflow - Replace drift-detection with affected-stacks and list-instances workflows - Add sha input, concurrency, ATMOS_PRO_WORKSPACE_ID, --upload-status to plan/apply - Use `atmos terraform deploy` (not apply --auto-approve) for apply workflow - Split single auth profile into github-plan and github-apply profiles - Add drift_detection detect/remediate and github_environment to mixin - Prevent command injection by passing inputs via env vars in run blocks - Organize templates into subfolders (workflows/, profiles/, mixins/, stacks/) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
April 6, 2026 21:53
Add OnConflict callback to installer options for handling file conflicts: - With --force: overwrite all files without prompting - Interactive TTY (no --force): prompt per-file via PromptForConfirmation - Non-TTY (no --force): error out via ErrInteractiveNotAvailable Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
pkg/pro/install/install.go (1)
141-146:⚠️ Potential issue | 🟡 MinorHandle
_defaults.yamlread failures explicitly in dry-run results.Right now, Line 141 read failures are reported as “skipped,” which hides real errors (e.g., permissions) and misleads the preview output.
Suggested adjustment
if i.writer.FileExists(defaultsPath) { content, err := i.writer.ReadFile(defaultsPath) - if err == nil && !hasImport(string(content), "mixins/atmos-pro") { + if err != nil { + // Surface read failure clearly in dry-run output strategy. + result.SkippedFiles = append(result.SkippedFiles, i.defaultsRelPath()) + // Consider extending InstallResult with FileErrors for precise reporting. + } else if !hasImport(string(content), "mixins/atmos-pro") { result.UpdatedFiles = append(result.UpdatedFiles, i.defaultsRelPath()) } else { result.SkippedFiles = append(result.SkippedFiles, i.defaultsRelPath()) } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/pro/install/install.go` around lines 141 - 146, The current logic treats any ReadFile error from i.writer.ReadFile(defaultsPath) as a skipped file; change it to handle read failures explicitly by checking if err != nil first and recording the failure in the dry-run result (e.g., append i.defaultsRelPath() to a result.Errors/FailedFiles slice or set result.Error with context) instead of result.SkippedFiles, and only use hasImport(string(content), "mixins/atmos-pro") to decide UpdatedFiles vs SkippedFiles when err == nil; modify the block around i.writer.ReadFile, hasImport, and i.defaultsRelPath() to reflect this explicit error path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/pro/install.go`:
- Around line 40-42: The current code returns a silent fallback ("." / "stacks")
whenever InitCliConfig returns an error; change this so InitCliConfig errors are
propagated to the caller (or a clear user-facing warning is emitted) instead of
using the default paths, except when the error explicitly indicates a "config
not found" condition; locate the call to InitCliConfig in the install flow (the
function handling install paths in cmd/pro/install.go), check for a specific
sentinel or typed error (e.g., ErrConfigNotFound) and only allow fallback in
that case, otherwise return the InitCliConfig error (or log a clear, actionable
message and return an error) so we don't write scaffolding into unintended
locations.
- Around line 31-35: Replace direct installCmd.Flags().BoolP(...) calls in
init() with a StandardParser: declare a package-level var installParser
*flags.StandardParser, initialize it in init() via flags.NewStandardParser(...)
using flags.WithBoolFlag for "yes" (short "y"), "force" (short "f") and
"dry-run" (no short), then call installParser.RegisterFlags(installCmd); remove
the BoolP bindings. In runInstall(), call
installParser.BindFlagsToViper(installCmd, v) to bind flags into Viper so flag
parsing follows the repo's standard pattern.
In `@pkg/pro/install/install.go`:
- Around line 273-279: The current code calls addImport(...) and then
unconditionally writes the updated file via i.writer.WriteFile, bypassing the
per-file conflict flow; change this to respect the installer's conflict policy
by invoking the existing OnConflict flow (or the installer option that handles
--force) before mutating the file: after computing updated := addImport(...),
call the installer conflict-check helper (e.g., OnConflict(relPath) or the
method that returns whether to overwrite when not forced), and only proceed to
i.writer.WriteFile(fullPath, ...), append to result.UpdatedFiles and return nil
if the conflict handler permits the overwrite (or if force is true); otherwise
skip writing and do not modify result.UpdatedFiles. Ensure you reference
addImport, i.writer.WriteFile, OnConflict (or the installer conflict helper),
fullPath, relPath, and result.UpdatedFiles in your change so the write respects
the user's conflict choice.
In `@pkg/pro/install/templates/profiles/README.md`:
- Around line 32-52: The example YAML blocks use a top-level identities: key but
the real schema expects auth.identities; update both example blocks so
identities is nested under an auth: mapping (i.e., replace the top-level
identities: with auth: then indent identities: under it), preserving the
existing identity name (<tenant>-<stage>/terraform), kind (aws/assume-role),
via/provider, and principal/assume_role fields so the examples match the
auth.identities schema.
In `@pkg/pro/install/templates/workflows/atmos-pro-terraform-plan.yaml`:
- Around line 15-17: The workflow defines an optional input named sha but later
uses it directly in the checkout action (ref: ${{ inputs.sha }}), which can be
empty and cause checkout to default to HEAD; either make the input required by
adding "sha: required: true" to the inputs block so callers must pass it, or
update the checkout reference to use a deterministic fallback (e.g., replace
uses of ref: ${{ inputs.sha }} with a safe expression that falls back to
github.sha when inputs.sha is empty) so the plan runs against the intended
commit; locate the inputs block containing "sha" and the checkout step
referencing inputs.sha to apply one of these fixes.
---
Duplicate comments:
In `@pkg/pro/install/install.go`:
- Around line 141-146: The current logic treats any ReadFile error from
i.writer.ReadFile(defaultsPath) as a skipped file; change it to handle read
failures explicitly by checking if err != nil first and recording the failure in
the dry-run result (e.g., append i.defaultsRelPath() to a
result.Errors/FailedFiles slice or set result.Error with context) instead of
result.SkippedFiles, and only use hasImport(string(content), "mixins/atmos-pro")
to decide UpdatedFiles vs SkippedFiles when err == nil; modify the block around
i.writer.ReadFile, hasImport, and i.defaultsRelPath() to reflect this explicit
error path.
🪄 Autofix (Beta)
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
Run ID: 6e0af561-14f8-4fa3-a246-d890f1733496
📒 Files selected for processing (15)
cmd/pro/install.gocmd/pro/markdown/atmos_pro_install_next_steps.mdpkg/pro/install/install.gopkg/pro/install/install_test.gopkg/pro/install/options.gopkg/pro/install/templates.gopkg/pro/install/templates/mixins/atmos-pro.yamlpkg/pro/install/templates/profiles/README.mdpkg/pro/install/templates/profiles/github-apply.yamlpkg/pro/install/templates/profiles/github-plan.yamlpkg/pro/install/templates/stacks/defaults-snippet.yamlpkg/pro/install/templates/workflows/atmos-pro-affected-stacks.yamlpkg/pro/install/templates/workflows/atmos-pro-list-instances.yamlpkg/pro/install/templates/workflows/atmos-pro-terraform-apply.yamlpkg/pro/install/templates/workflows/atmos-pro-terraform-plan.yaml
✅ Files skipped from review due to trivial changes (8)
- pkg/pro/install/templates/stacks/defaults-snippet.yaml
- pkg/pro/install/templates/profiles/github-apply.yaml
- pkg/pro/install/templates/mixins/atmos-pro.yaml
- cmd/pro/markdown/atmos_pro_install_next_steps.md
- pkg/pro/install/templates/profiles/github-plan.yaml
- pkg/pro/install/templates/workflows/atmos-pro-terraform-apply.yaml
- pkg/pro/install/templates/workflows/atmos-pro-affected-stacks.yaml
- pkg/pro/install/templates/workflows/atmos-pro-list-instances.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/pro/install/options.go
- pkg/pro/install/install_test.go
- pkg/pro/install/templates.go
Fix lock flag help text to clarify effective defaults without changing sentinel values, fix unlock flag descriptions (copy-paste "lock" → "unlock"), and handle ReadFile error explicitly in DryRun. Add WithForce() to StandardOptionsBuilder for consistent --force flag across commands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
|
Note Release Documentation Complete ✅
Thank you! |
|
Warning Release Documentation RequiredThis PR is labeled
|
Refactor install flags to use flags.NewStandardParser() with Viper integration for proper CLI > ENV > config precedence. Surface warning when atmos config fails to load instead of silent fallback. Respect OnConflict policy before mutating existing _defaults.yaml. Fix profile README examples to match auth.identities schema. Add sha fallback in workflow checkout steps to prevent empty-ref defaulting to HEAD. Add blog post and roadmap entry for pro install command. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
force-pushed
the
osterman/pro-install-cmd
branch
from
April 7, 2026 00:28
05e3b12 to
c454663
Compare
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (70.05%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2288 +/- ##
==========================================
- Coverage 80.58% 80.53% -0.06%
==========================================
Files 1524 1529 +5
Lines 144122 145142 +1020
==========================================
+ Hits 116142 116888 +746
- Misses 21466 21666 +200
- Partials 6514 6588 +74
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…and roadmap - Replace hardcoded deploy/_defaults.yaml with .atmos.d/ci.yaml and .atmos.d/atmos-pro.yaml drop-in configuration files - Create minimal atmos.yaml when missing as anchor for drop-in configs - Add documentation page for atmos pro install command - Add blog post and roadmap entry for the feature - Fix next steps links to use atmos-pro.com URLs consistently - Wrap filenames in backticks in status output - Add blank line before next steps markdown Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add browser prompt after installation to open workspace creation page - Update ci.yaml template with full output/summary/checks config - Simplify atmos-pro.yaml to commented workspace_id with !env reference - Fix next steps to use consistent "See: ..." link style Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add OSFileWriter tests (WriteFile, ReadFile, FileExists, MkdirAll) - Add error path tests (MkdirAll failure, WriteFile failure) - Add DryRun with existing files and force override tests - Add custom stacks path test - Add WithOnConflict option test - Add reportResult and reportDryRun tests - Add ProCommandProvider nil-return method tests - Add embedded markdown content tests - pkg/pro/install coverage: 100% Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
cmd/pro/pro_test.go (1)
153-192: Assert the rendered report, not just “no panic”.These subtests will miss regressions in created/updated/skipped reporting because they never inspect what
reportResultorreportDryRunactually emit. Capture the UI stream, or route the formatter through an injected writer, and assert the rendered lines instead.As per coding guidelines, "Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable via dependency injection. No coverage theater."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/pro/pro_test.go` around lines 153 - 192, The tests for reportResult and reportDryRun only assert “no panic” and miss regressions in the actual output; update the tests to capture and assert the rendered report text by routing the formatter to a test writer or capturing the UI output instead of only calling reportResult/reportDryRun. Specifically, modify TestReportResult and TestReportDryRun to provide a bytes.Buffer (or captured UI stream) to the code path used by reportResult and reportDryRun, call those functions with the InstallResult cases, and assert that the buffer contains the expected lines for CreatedFiles, UpdatedFiles, and SkippedFiles (and the empty-result case) so the tests verify actual rendered output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/pro/install.go`:
- Around line 76-78: The post-install workspace prompt is invoked
unconditionally via promptOpenWorkspace() even when the --yes flag (variable
yes) is set; update the code to skip or short-circuit promptOpenWorkspace when
yes is true (either by gating calls with if !yes { promptOpenWorkspace(...) } or
by passing the yes flag into promptOpenWorkspace and handling it there), and
apply the same change to the other promptOpenWorkspace invocations in this file
so interactive runs with --yes do not stop for an extra prompt.
- Around line 52-67: resolveInstallPaths currently calls cfg.InitCliConfig with
an empty schema.ConfigAndStacksInfo which ignores global flags; update
runInstall to call flags.ParseGlobalFlags(cmd, &cfgInfo) to populate a
schema.ConfigAndStacksInfo variable (e.g., cfgInfo) and then pass that populated
cfgInfo into cfg.InitCliConfig instead of an empty struct, ensuring
resolveInstallPaths (and the similar block around lines 70-80) use the parsed
global flags when resolving basePath and stacksBasePath; keep function names:
resolveInstallPaths, runInstall, cfg.InitCliConfig, schema.ConfigAndStacksInfo,
and flags.ParseGlobalFlags to locate the changes.
In `@pkg/flags/standard_builder.go`:
- Around line 164-172: The WithForce() builder registers a duplicate shorthand
"-f" colliding with WithFormat(); update StandardOptionsBuilder.WithForce to
stop passing the shorthand to WithBoolFlag (use no shorthand / empty string) so
it only registers the long name "force" and env var via WithEnvVars("force",
"ATMOS_FORCE"); leave perf.Track, the append to b.options, and WithEnvVars
intact.
In `@pkg/pro/install/install.go`:
- Around line 117-129: DryRun and Install currently append paths to
InstallResult.CreatedFiles even when an existing file will be overwritten;
populate InstallResult.UpdatedFiles instead for existing files that will be
modified. Update Installer.DryRun (where you check i.writer.FileExists(fullPath)
&& !i.opts.Force) to append f.RelPath to result.SkippedFiles when not forcing,
but when forcing append to result.UpdatedFiles (not CreatedFiles); do the
analogous change in Installer.Install where confirmed overwrites or --force
cause writes (use InstallResult.UpdatedFiles rather than CreatedFiles) and keep
creating new paths in CreatedFiles only when the file did not previously exist.
Ensure checks reference writer.FileExists, opts.Force, and any confirmation flow
used in Install.
- Around line 139-144: The root config entry currently returned in the slice as
a fileSpec with RelPath "atmos.yaml" is treated like other scaffolded files and
gets routed through writeFile(), which allows overwriting an existing user
config; change the behavior so "atmos.yaml" is marked create-only and skipped if
the file already exists. Update the fileSpec for atmos.yaml (and the other
similar entries around the 193-205 range) to include a create-only policy flag
or a special field checked by writeFile(), then modify writeFile() to detect
that flag and return without prompting or writing when the destination exists;
reference the fileSpec construction for atmos.yaml and the writeFile() function
to implement this guard.
---
Nitpick comments:
In `@cmd/pro/pro_test.go`:
- Around line 153-192: The tests for reportResult and reportDryRun only assert
“no panic” and miss regressions in the actual output; update the tests to
capture and assert the rendered report text by routing the formatter to a test
writer or capturing the UI output instead of only calling
reportResult/reportDryRun. Specifically, modify TestReportResult and
TestReportDryRun to provide a bytes.Buffer (or captured UI stream) to the code
path used by reportResult and reportDryRun, call those functions with the
InstallResult cases, and assert that the buffer contains the expected lines for
CreatedFiles, UpdatedFiles, and SkippedFiles (and the empty-result case) so the
tests verify actual rendered output.
🪄 Autofix (Beta)
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
Run ID: 6c27f3d3-6e4e-40ac-b2e7-ad93d23a8359
📒 Files selected for processing (21)
cmd/pro/install.gocmd/pro/lock.gocmd/pro/markdown/atmos_pro_install.mdcmd/pro/markdown/atmos_pro_install_next_steps.mdcmd/pro/pro_test.gocmd/pro/unlock.gopkg/flags/standard_builder.gopkg/flags/standard_options.gopkg/flags/standard_parser.gopkg/pro/install/install.gopkg/pro/install/install_test.gopkg/pro/install/templates.gopkg/pro/install/templates/atmos.d/atmos-pro.yamlpkg/pro/install/templates/atmos.d/ci.yamlpkg/pro/install/templates/atmos.yamlpkg/pro/install/templates/profiles/README.mdpkg/pro/install/templates/workflows/atmos-pro-terraform-apply.yamlpkg/pro/install/templates/workflows/atmos-pro-terraform-plan.yamlwebsite/blog/2026-04-06-pro-install-command.mdxwebsite/docs/cli/commands/pro/pro-install.mdxwebsite/src/data/roadmap.js
✅ Files skipped from review due to trivial changes (8)
- pkg/pro/install/templates/atmos.yaml
- pkg/pro/install/templates/atmos.d/atmos-pro.yaml
- pkg/pro/install/templates/atmos.d/ci.yaml
- website/blog/2026-04-06-pro-install-command.mdx
- cmd/pro/markdown/atmos_pro_install_next_steps.md
- website/src/data/roadmap.js
- pkg/pro/install/templates/workflows/atmos-pro-terraform-plan.yaml
- pkg/pro/install/templates/workflows/atmos-pro-terraform-apply.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/pro/unlock.go
- pkg/pro/install/templates.go
Address CodeRabbit review on flag default values by extracting named constants to satisfy the add-constant revive linter rule. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
Resolve conflict in roadmap.js by keeping both pro-commit and pro-install milestones. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oCmd The file was in package cmd but referenced proCmd which is defined in package pro (cmd/pro/pro.go), causing build failures on all platforms. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use absolute paths and log warnings instead of assert for cleanup errors, matching the robust pattern already in TestProcessCustomYamlTags. Add pre-test cleanup to remove stale state from prior test runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
# Conflicts: # internal/exec/stack_processor_utils.go # pkg/config/cache_lock_unix.go
Contributor
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
# Conflicts: # internal/exec/yaml_func_terraform_state_test.go
Contributor
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
This branch was successfully deployed
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
atmos pro installcommand that scaffolds Atmos Pro configuration files into a projectprocommand from legacy direct-registration pattern to theCommandProviderregistry pattern (likecmd/ci/,cmd/ai/)profiles/github/atmos.yamlauth profile with OIDC placeholder valuesstacks/mixins/atmos-pro.yamlmixin and updatestacks/deploy/_defaults.yaml--yes,--force,--dry-runflags with interactive charm/huh confirmation promptswhy
procommand was the last remaining command using the old registration pattern — this brings it in line with the modernCommandProviderarchitecturereferences
Summary by CodeRabbit
New Features
Documentation
Chores