feat: Add Terraform streaming UI with real-time visualization#1908
feat: Add Terraform streaming UI with real-time visualization#1908
Conversation
|
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughAdds a Terraform streaming TUI: new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI (cobra)
participant Parser as Viper/Options Parser
participant Exec as internal/exec
participant Detector as ShouldUseStreamingUI
participant TFUI as pkg/terraform/ui
participant Shell as Shell Executor
participant StreamParser as UI JSON Parser
participant Tracker as ResourceTracker
participant UI as Bubble Tea Model
User->>CLI: atmos terraform apply --ui
CLI->>Parser: read flags & config (viper + cobra)
Parser->>Exec: terraformRunWithOptions(opts)
Exec->>Detector: ShouldUseStreamingUI(opts)
alt streaming enabled
Detector-->>Exec: true
Exec->>TFUI: ExecuteApply with -json
TFUI->>StreamParser: stream stdout lines
loop per JSON line
StreamParser->>Tracker: HandleMessage(msg)
Tracker->>UI: update state
UI->>UI: render progress
end
TFUI-->>Exec: return exit/result
else streaming unsupported
Detector-->>Exec: ErrStreamingNotSupported
Exec->>Shell: fallback terraform CLI
Shell-->>Exec: return exit/result
else streaming disabled
Detector-->>Exec: false
Exec->>Shell: run terraform normally
Shell-->>Exec: return exit/result
end
Exec-->>CLI: exit code/result
CLI-->>User: display outcome
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
pkg/terraform/ui/executor.go (2)
136-163: buildArgsWithJSON allocation size warning is a false positive.The
len(args)+1capacity calculation is safe—CLI argument counts are OS-bounded (typically a few thousand max). CodeQL flags this theoretically but it's not exploitable in practice.
551-571: buildPlanArgs and buildDestroyPlanArgs allocation warnings are false positives.Same reasoning as line 146—CLI argument counts are bounded.
Also applies to: 578-598
🧹 Nitpick comments (10)
cmd/terraform/options_test.go (1)
36-53: Consider adding UI flag test coverage.The test cases don't yet cover the new
UIandUIFlagSetfields. You might want to add a test case that validatesv.Set("ui", true)results inopts.UI == true, and a separate test with a mock command to verifyUIFlagSetbehavior.pkg/terraform/ui/confirm.go (1)
36-55: Consider extracting a shared helper to reduce duplication.Both
ConfirmApplyandConfirmDestroyhave identical logic differing only by the title string. A shared helper would reduce maintenance burden.🔎 Proposed refactor
+// confirmPrompt is the shared implementation for confirmation prompts. +func confirmPrompt(title string) (bool, error) { + var confirm bool + theme := uiutils.NewAtmosHuhTheme() + + prompt := huh.NewConfirm(). + Title(title). + Affirmative("Yes"). + Negative("No"). + Value(&confirm). + WithButtonAlignment(lipgloss.Left). + WithTheme(theme) + + if err := prompt.Run(); err != nil { + if errors.Is(err, huh.ErrUserAborted) { + return false, errUtils.ErrUserAborted + } + return false, err + } + return confirm, nil +} + // ConfirmApply prompts the user to confirm applying changes. func ConfirmApply() (bool, error) { - var confirm bool - theme := uiutils.NewAtmosHuhTheme() - - prompt := huh.NewConfirm(). - Title("Do you want to apply these changes?"). - Affirmative("Yes"). - Negative("No"). - Value(&confirm). - WithButtonAlignment(lipgloss.Left). - WithTheme(theme) - - if err := prompt.Run(); err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return false, errUtils.ErrUserAborted - } - return false, err - } - return confirm, nil + return confirmPrompt("Do you want to apply these changes?") } // ConfirmDestroy prompts the user to confirm destroying resources. func ConfirmDestroy() (bool, error) { - var confirm bool - theme := uiutils.NewAtmosHuhTheme() - - prompt := huh.NewConfirm(). - Title("Do you want to destroy these resources?"). - Affirmative("Yes"). - Negative("No"). - Value(&confirm). - WithButtonAlignment(lipgloss.Left). - WithTheme(theme) - - if err := prompt.Run(); err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return false, errUtils.ErrUserAborted - } - return false, err - } - return confirm, nil + return confirmPrompt("Do you want to destroy these resources?") }internal/exec/terraform.go (1)
596-675: Consider extracting workspace options construction.The
ExecuteOptionssetup for workspace select (lines 597-607) and workspace new (lines 641-651) are nearly identical, differing only inArgs. A helper could reduce duplication, though this is minor.🔎 Optional: Extract helper
func buildWorkspaceOpts(info *schema.ConfigAndStacksInfo, componentPath string, action string, workspace string) *tfui.ExecuteOptions { return &tfui.ExecuteOptions{ Command: info.Command, Args: []string{"workspace", action, workspace}, WorkingDir: componentPath, Env: info.ComponentEnvList, Component: info.FinalComponent, Stack: info.Stack, SubCommand: "workspace", Workspace: workspace, DryRun: info.DryRun, } }pkg/terraform/ui/tree_test.go (1)
113-128: sortChildren test covers basic sorting but misses recursive case.The test validates top-level sorting. Consider adding a test case with nested children to verify
sortChildrensorts recursively if that's expected behavior.🔎 Optional: Add recursive sorting test
func TestSortChildren_Recursive(t *testing.T) { root := &TreeNode{ Address: "root", Children: []*TreeNode{ { Address: "z_resource", Children: []*TreeNode{ {Address: "z_child"}, {Address: "a_child"}, }, }, {Address: "a_resource"}, }, } sortChildren(root) assert.Equal(t, "a_resource", root.Children[0].Address) assert.Equal(t, "z_resource", root.Children[1].Address) // Verify nested children are also sorted. assert.Equal(t, "a_child", root.Children[1].Children[0].Address) assert.Equal(t, "z_child", root.Children[1].Children[1].Address) }pkg/terraform/ui/executor_test.go (1)
103-120: ExecuteOptions test is somewhat tautological.This test only verifies that struct field assignment works, which Go guarantees. Consider testing actual behavior that uses ExecuteOptions instead, or remove if there's no meaningful behavior to validate.
pkg/terraform/ui/model.go (2)
28-44: Unusedreaderfield in Model struct.The
readeris passed toNewParserin the constructor and stored inm.reader, but it's never used after that. The parser owns the reader.🔎 Remove unused field
type Model struct { tracker *ResourceTracker parser *Parser - reader io.Reader spinner spinner.Model progress progress.Model width intAnd in NewModel:
return &Model{ tracker: NewResourceTracker(), parser: NewParser(reader), - reader: reader, spinner: s,
247-268: Consider consolidating action verb formatting.
formatActivityVerb,formatActionPending,formatActionInProgress, andformatActionCompleteall map the same actions to slightly different verb forms. A single lookup table or helper could reduce duplication.🔎 Consolidated approach
var actionVerbs = map[string]struct { pending, inProgress, complete string }{ "create": {"Create", "Creating", "Created"}, "read": {"Read", "Reading", "Read"}, "update": {"Update", "Updating", "Updated"}, "delete": {"Destroy", "Destroying", "Destroyed"}, "no-op": {"No change", "No change", "No change"}, } func getActionVerb(action string, form int) string { if v, ok := actionVerbs[action]; ok { switch form { case 0: return v.pending case 1: return v.inProgress case 2: return v.complete } } return action }Also applies to: 325-377
pkg/terraform/ui/resource_test.go (1)
306-339: Concurrency test has a subtle address collision issue.The expression
string(rune('0'+n%10))produces only 10 unique addresses (test_0throughtest_9) for 50 goroutines. This means resources are overwritten rather than added, making the final count unpredictable. If the intent is stress-testing concurrent writes to the same keys, that's valid—but the assertionassert.Greater(t, rt.GetTotalCount(), 0)is weak. If the intent is 50 unique resources, fix the address generation.Option: Use unique addresses per goroutine
go func(n int) { rt.HandleMessage(&PlannedChangeMessage{ Change: PlannedChange{ - Resource: ResourceAddr{Addr: "aws_instance.test_" + string(rune('0'+n%10))}, + Resource: ResourceAddr{Addr: fmt.Sprintf("aws_instance.test_%d", n)}, Action: "create", }, }) done <- true }(i)This would require adding
"fmt"to imports and would result in 50 unique resources.pkg/terraform/ui/parser.go (1)
34-56: Consider iterative empty-line skip instead of recursion.The recursive
p.Next()call on line 47 works fine in practice, but deeply malformed input with many consecutive empty lines could cause stack growth. A loop would be more robust.Iterative approach
func (p *Parser) Next() (*ParseResult, error) { + for { if !p.scanner.Scan() { if err := p.scanner.Err(); err != nil { return nil, err } return nil, io.EOF } line := p.scanner.Bytes() if len(line) == 0 { - // Skip empty lines. - return p.Next() + continue } msg, err := p.parseMessage(line) return &ParseResult{ Message: msg, Raw: append([]byte{}, line...), // Copy to avoid scanner reuse. Err: err, }, nil + } }pkg/terraform/ui/resource.go (1)
202-207: GetChangeSummary and GetOutputs return internal pointers.Unlike
GetResourcesandGetDiagnostics, these return pointers to internal state without copying. If callers mutate the returned structs, it could corrupt tracker state. Consider returning copies or documenting as read-only.Return copies for safety
func (rt *ResourceTracker) GetChangeSummary() *ChangeSummaryMessage { rt.mu.RLock() defer rt.mu.RUnlock() + if rt.changeSummary == nil { + return nil + } + copy := *rt.changeSummary - return rt.changeSummary + return © }Also applies to: 304-309
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/tui/utils/utils.go (1)
41-147: Add periods to inline comments.Several inline comments are missing trailing periods (lines 41, 78, 79, 88, 89, 110, 115, 118, 124, 130, 136, 141, 145, 147), which violates the godot linter requirement.
As per coding guidelines, all comments must end with periods.
♻️ Duplicate comments (2)
pkg/terraform/ui/executor.go (2)
136-163: Solid argument construction logic.The function correctly handles the -json flag insertion. The static analysis overflow warning on line 146 is a theoretical concern - CLI argument slices are bounded by practical limits.
551-598: Argument transformation logic is correct.Both functions properly convert between command types and handle flag filtering. The static analysis overflow warnings (lines 553, 580) are low-risk given CLI argument constraints.
🧹 Nitpick comments (16)
internal/tui/utils/utils.go (1)
31-162: Consider adding performance tracking to public functions.All public functions in this file are missing
defer perf.Track()calls. Per coding guidelines, public functions should include performance tracking.Based on coding guidelines, public functions should have performance tracking.
cmd/terraform/workspace.go (1)
44-45: Consider updating the comment for consistency.The call site is correct, but the comment on line 44 is generic. For consistency with
apply.goanddeploy.go, consider updating it to mention the command context purpose:- // Parse base terraform options. + // Parse base terraform options with command context for UI flag detection. opts := ParseTerraformRunOptions(v, cmd)pkg/terraform/ui/confirm.go (2)
14-33: Missingperf.Track()on public functions.Per coding guidelines, public functions should include performance tracking. Consider adding:
func ConfirmApply() (bool, error) { + defer perf.Track(nil, "ui.ConfirmApply")() + var confirm boolSame applies to
ConfirmDestroy().
35-55: Consider extracting shared confirmation logic.Both
ConfirmApplyandConfirmDestroyshare identical structure. A private helper could reduce duplication:🔎 Optional refactor
func confirmPrompt(title string) (bool, error) { defer perf.Track(nil, "ui.confirmPrompt")() var confirm bool theme := uiutils.NewAtmosHuhTheme() prompt := huh.NewConfirm(). Title(title). Affirmative("Yes"). Negative("No"). Value(&confirm). WithButtonAlignment(lipgloss.Left). WithTheme(theme) if err := prompt.Run(); err != nil { if errors.Is(err, huh.ErrUserAborted) { return false, errUtils.ErrUserAborted } return false, err } return confirm, nil }pkg/terraform/ui/executor_test.go (2)
9-15: Test doesn't validate intended behavior.This test expects
falsebecause it runs in CI. The comment acknowledges this limitation. Consider either:
- Mocking
telemetry.IsCI()andterm.IsTTYSupportForStdout()to test the positive case- Removing this test if it can't validate the expected behavior
As-is, it only confirms CI auto-disables the feature, not that explicit enable works.
103-120: Tautological test - consider removal.This test only verifies that struct field assignment works. Per coding guidelines, avoid tautological tests that test language mechanics rather than behavior.
internal/exec/terraform.go (1)
588-681: Consider extracting workspace streaming helper to reduce duplication.The streaming patterns for workspace select (lines 596-631) and workspace new (lines 640-675) are nearly identical. A helper function could consolidate this:
🔎 Optional refactor
func executeWorkspaceWithStreaming( useStreaming bool, atmosConfig schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, args []string, componentPath string, redirectStdErr string, ) error { if useStreaming { opts := &tfui.ExecuteOptions{ Command: info.Command, Args: args, WorkingDir: componentPath, Env: info.ComponentEnvList, Component: info.FinalComponent, Stack: info.Stack, SubCommand: "workspace", Workspace: info.TerraformWorkspace, DryRun: info.DryRun, } err := tfui.ExecuteInit(context.Background(), opts) if !errors.Is(err, errUtils.ErrStreamingNotSupported) { return err } } return ExecuteShellCommand( atmosConfig, info.Command, args, componentPath, info.ComponentEnvList, info.DryRun, redirectStdErr, ) }pkg/terraform/ui/parser.go (1)
16-25: Missingperf.Track()on public function.Per coding guidelines, public functions should include performance tracking:
func NewParser(r io.Reader) *Parser { + defer perf.Track(nil, "ui.NewParser")() + scanner := bufio.NewScanner(r)pkg/terraform/ui/init_model.go (1)
54-70: Missingperf.Track()on public constructor.func NewInitModel(component, stack, subCommand, workspace string, reader io.Reader) *InitModel { + defer perf.Track(nil, "ui.NewInitModel")() + // Use MiniDot spinner for init/workspace (more subtle, different from plan/apply).pkg/terraform/ui/tree.go (3)
55-71: Missingperf.Track()on public function.Per coding guidelines:
func BuildDependencyTree(ctx context.Context, planfilePath, terraformPath, workingDir, stack, component string) (*DependencyTree, error) { + defer perf.Track(nil, "ui.BuildDependencyTree")() + // Run terraform show -json planfile.Also consider capturing stderr for better error diagnostics:
- output, err := cmd.Output() + output, err := cmd.Output() if err != nil { - return nil, fmt.Errorf("failed to run terraform show: %w", err) + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && len(exitErr.Stderr) > 0 { + return nil, fmt.Errorf("failed to run terraform show: %w: %s", err, string(exitErr.Stderr)) + } + return nil, fmt.Errorf("failed to run terraform show: %w", err) }
253-268: Missingperf.Track()on public method.func (t *DependencyTree) RenderTree() string { + defer perf.Track(nil, "ui.DependencyTree.RenderTree")() + var b strings.Builder
786-803: Consider usingstrconv.ParseIntfor hex parsing.The manual parsing works but
strconv.ParseInt(hex, 16, 64)is simpler and handles edge cases:🔎 Optional simplification
func parseHexComponent(hex string) (int64, error) { - var result int64 - for _, c := range hex { - result *= 16 - switch { - case c >= '0' && c <= '9': - result += int64(c - '0') - case c >= 'a' && c <= 'f': - result += int64(c - 'a' + 10) - case c >= 'A' && c <= 'F': - result += int64(c - 'A' + 10) - default: - return 0, fmt.Errorf("%w: invalid hex character: %c", errUtils.ErrParseHexColor, c) - } - } - return result, nil + result, err := strconv.ParseInt(hex, 16, 64) + if err != nil { + return 0, fmt.Errorf("%w: %v", errUtils.ErrParseHexColor, err) + } + return result, nil }pkg/terraform/ui/executor.go (4)
113-116: Type assertion could panic if model type changes.The assertion
finalModel.(Model)will panic ifp.Run()returns an unexpected type. Consider using the comma-ok idiom for defensive coding.Suggested fix
- m := finalModel.(Model) - if m.GetError() != nil { - return m.GetError() + m, ok := finalModel.(Model) + if !ok { + return fmt.Errorf("unexpected model type from TUI: %T", finalModel) + } + if m.GetError() != nil { + return m.GetError() }
288-291: Same type assertion pattern - apply consistent fix.Same issue as in
Execute()- use comma-ok idiom for safety.Suggested fix
- m := finalModel.(InitModel) - if m.GetError() != nil { - return m.GetError() + m, ok := finalModel.(InitModel) + if !ok { + return fmt.Errorf("unexpected model type from TUI: %T", finalModel) + } + if m.GetError() != nil { + return m.GetError() }
632-641: Consider using CommandContext for timeout safety.
exec.Commandwithout context could hang indefinitely ifterraform outputblocks. Since this is called in the success path, a hung process would freeze the UI.Suggested fix
-func fetchAndDisplayOutputs(command, workingDir string) { +func fetchAndDisplayOutputs(ctx context.Context, command, workingDir string) { // Run terraform output -json to get current outputs. - cmd := exec.Command(command, "output", "-json") + // Use a timeout context to prevent hanging on unresponsive terraform. + timeoutCtx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + cmd := exec.CommandContext(timeoutCtx, command, "output", "-json") cmd.Dir = workingDirThen update call sites to pass the context.
1-26: File organization is acceptable.The file is slightly over the 600-line guideline, but functions are cohesive. Consider extracting output display logic (lines 600-815) to a separate file like
outputs.goin a future refactor if the file grows further.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/terraform/ui/parser.go (1)
62-68: Consider using static error from errors/errors.go.The error at line 67 uses a dynamic string. Per coding guidelines, errors should be wrapped using static errors defined in
errors/errors.go.Suggested approach
// In errors/errors.go, add: // ErrInvalidJSON = errors.New("invalid JSON") // Then in parser.go: return nil, fmt.Errorf("%w: %w", errUtils.ErrInvalidJSON, err)docs/prd/terraform-streaming-ui.md (1)
38-44: Fenced code blocks missing language specifiers.Multiple code blocks lack language identifiers (lines 38, 48, 61, 72, 78, 83, 167). Per learnings, this can be addressed in a separate documentation cleanup commit.
Based on learnings, deferring MD040 fixes to a follow-up PR is acceptable.
Also applies to: 48-56, 61-69, 72-75, 78-80, 83-86, 167-178
📜 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 (8)
docs/prd/terraform-streaming-ui.mdpkg/terraform/ui/executor.gopkg/terraform/ui/executor_test.gopkg/terraform/ui/init_model.gopkg/terraform/ui/parser.gopkg/terraform/ui/tree.gopkg/terraform/ui/tree_test.gopkg/terraform/ui/types.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/terraform/ui/executor_test.go
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/terraform/ui/tree_test.gopkg/terraform/ui/parser.gopkg/terraform/ui/types.gopkg/terraform/ui/tree.gopkg/terraform/ui/init_model.gopkg/terraform/ui/executor.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/terraform/ui/tree_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/terraform-streaming-ui.md
🧠 Learnings (37)
📓 Common learnings
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.
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: website/docs/cheatsheets/vendoring.mdx:70-70
Timestamp: 2024-11-12T13:06:56.194Z
Learning: In `atmos vendor pull --everything`, the `--everything` flag uses the TTY for TUI but is not interactive.
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.
📚 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/terraform/ui/tree_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/terraform/ui/tree_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/terraform/ui/tree_test.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/terraform/ui/tree_test.gopkg/terraform/ui/types.gopkg/terraform/ui/tree.gopkg/terraform/ui/init_model.gopkg/terraform/ui/executor.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/terraform/ui/tree_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/terraform/ui/tree_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 : Use colors from pkg/ui/theme/colors.go for all UI output
Applied to files:
pkg/terraform/ui/tree_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/terraform/ui/tree_test.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/terraform/ui/tree_test.gopkg/terraform/ui/parser.gopkg/terraform/ui/types.gopkg/terraform/ui/tree.gopkg/terraform/ui/init_model.gopkg/terraform/ui/executor.go
📚 Learning: 2025-01-09T22:22:00.539Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/helmfile_destroy.go:6-15
Timestamp: 2025-01-09T22:22:00.539Z
Learning: Usage commands should not be added to helmfile subcommands (destroy, apply, sync, diff) as the usage is handled by the parent helmfile command.
Applied to files:
docs/prd/terraform-streaming-ui.md
📚 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:
docs/prd/terraform-streaming-ui.mdpkg/terraform/ui/executor.go
📚 Learning: 2024-10-21T17:51:07.087Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
Applied to files:
docs/prd/terraform-streaming-ui.md
📚 Learning: 2024-11-10T18:37:10.032Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_component_utils.go:354-360
Timestamp: 2024-11-10T18:37:10.032Z
Learning: In the vendoring process, a TTY can exist without being interactive. If the process does not prompt the user, we should not require interactive mode to display the TUI. The `CheckTTYSupport` function should check TTY support on stdout rather than stdin.
Applied to files:
docs/prd/terraform-streaming-ui.md
📚 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/terraform-streaming-ui.md
📚 Learning: 2024-10-30T13:25:45.965Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:405-412
Timestamp: 2024-10-30T13:25:45.965Z
Learning: In `internal/exec/terraform_clean.go`, when appending `stackFolders` to `folders` in the `handleCleanSubCommand` function, it's unnecessary to check if `stackFolders` is nil before appending, because in Go, appending a nil slice is safe and does not cause a panic.
Applied to files:
pkg/terraform/ui/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/terraform/ui/parser.gopkg/terraform/ui/init_model.gopkg/terraform/ui/executor.go
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
pkg/terraform/ui/parser.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/terraform/ui/parser.gopkg/terraform/ui/types.gopkg/terraform/ui/tree.gopkg/terraform/ui/init_model.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/terraform/ui/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/terraform/ui/parser.go
📚 Learning: 2024-10-27T04:28:40.966Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:155-175
Timestamp: 2024-10-27T04:28:40.966Z
Learning: In the `CollectDirectoryObjects` function in `internal/exec/terraform_clean.go`, recursive search through all subdirectories is not needed.
Applied to files:
pkg/terraform/ui/parser.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
pkg/terraform/ui/parser.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/terraform/ui/parser.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: Separate I/O (streams) from UI (formatting): use I/O Layer (pkg/io/) for stream access and UI Layer (pkg/ui/) for formatting; use data.Write/Writeln/WriteJSON/WriteYAML for pipeable output to stdout and ui.Write/Success/Error/Warning/Info for human messages to stderr
Applied to files:
pkg/terraform/ui/parser.gopkg/terraform/ui/types.gopkg/terraform/ui/executor.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
pkg/terraform/ui/types.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 comments must end with periods (enforced by godot linter)
Applied to files:
pkg/terraform/ui/types.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:
pkg/terraform/ui/types.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:
pkg/terraform/ui/types.gopkg/terraform/ui/tree.gopkg/terraform/ui/init_model.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/terraform/ui/tree.go
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
pkg/terraform/ui/tree.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/terraform/ui/init_model.go
📚 Learning: 2024-11-01T14:45:32.417Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 757
File: cmd/docs.go:52-54
Timestamp: 2024-11-01T14:45:32.417Z
Learning: In `cmd/docs.go`, capping the terminal width at 120 columns is considered acceptable and preferred after testing.
Applied to files:
pkg/terraform/ui/init_model.go
📚 Learning: 2024-11-01T15:44:12.617Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 757
File: cmd/docs.go:42-59
Timestamp: 2024-11-01T15:44:12.617Z
Learning: In `cmd/docs.go`, when implementing width detection for the `docsCmd` command, it's acceptable to keep the code inline without extracting it into a separate function, as per the user's preference for compact readability and maintainability in Go code.
Applied to files:
pkg/terraform/ui/init_model.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/terraform/ui/init_model.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/terraform/ui/init_model.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:
pkg/terraform/ui/init_model.go
🧬 Code graph analysis (3)
pkg/terraform/ui/tree_test.go (1)
pkg/terraform/ui/tree.go (2)
DependencyTree(29-34)TreeNode(37-44)
pkg/terraform/ui/parser.go (1)
pkg/terraform/ui/types.go (27)
BaseMessage(32-38)MessageTypeVersion(15-15)VersionMessage(41-45)MessageTypePlannedChange(16-16)PlannedChangeMessage(67-70)MessageTypeChangeSummary(17-17)ChangeSummaryMessage(163-166)MessageTypeApplyStart(18-18)ApplyStartMessage(82-85)MessageTypeApplyProgress(19-19)ApplyProgressMessage(88-91)MessageTypeApplyComplete(20-20)ApplyCompleteMessage(94-97)MessageTypeApplyErrored(21-21)ApplyErroredMessage(100-103)MessageTypeRefreshStart(22-22)RefreshStartMessage(113-116)MessageTypeRefreshComplete(23-23)RefreshCompleteMessage(119-122)MessageTypeDiagnostic(24-24)DiagnosticMessage(148-151)MessageTypeOutputs(25-25)OutputsMessage(177-180)MessageTypeInitOutput(27-27)InitOutputMessage(183-189)MessageTypeLog(28-28)LogMessage(192-195)
pkg/terraform/ui/init_model.go (3)
pkg/ui/spinner/spinner.go (1)
Spinner(267-272)pkg/ui/theme/colors.go (2)
ColorCyan(32-32)ColorGray(30-30)pkg/ui/formatter.go (2)
FormatErrorf(341-343)FormatSuccessf(324-326)
🪛 LanguageTool
docs/prd/terraform-streaming-ui.md
[typographical] ~17-~17: Consider using a typographic opening quote here.
Context: ...ation of progress. Users see occasional "Still creating..." messages but have no ...
(EN_QUOTES)
[style] ~17-~17: Consider using the typographical ellipsis character here instead.
Context: ...ss. Users see occasional "Still creating..." messages but have no sense of completi...
(ELLIPSIS)
[typographical] ~17-~17: Consider using a typographic close quote here.
Context: ... Users see occasional "Still creating..." messages but have no sense of completio...
(EN_QUOTES)
[style] ~117-~117: Consider using a more formal/concise alternative here.
Context: ...et 3. Unsupported command: Commands other than plan, apply, init, destroy Thi...
(OTHER_THAN)
[typographical] ~135-~135: To join two clauses or introduce examples, consider using an em dash.
Context: ... streaming format includes: - version - Terraform version info - `planned_change...
(DASH_RULE)
[typographical] ~137-~137: To join two clauses or introduce examples, consider using an em dash.
Context: ...ss/apply_complete/apply_errored- Apply lifecycle -refresh_start/ref...
(DASH_RULE)
[typographical] ~138-~138: To join two clauses or introduce examples, consider using an em dash.
Context: ...e - refresh_start / refresh_complete - State refresh lifecycle - `change_summar...
(DASH_RULE)
[typographical] ~146-~146: To join two clauses or introduce examples, consider using an em dash.
Context: ...odes are critical for automation: - 0 - Success - 1 - Error - 2 - Success wi...
(DASH_RULE)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/terraform-streaming-ui.md
38-38: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
61-61: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
72-72: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
83-83: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
167-167: 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). (1)
- GitHub Check: Summary
🔇 Additional comments (28)
pkg/terraform/ui/tree_test.go (4)
11-32: Good table-driven test structure for action symbols.Clean use of table-driven testing as per guidelines. The test correctly validates that colored output contains expected symbols across all action types including the composite "replace" action.
34-54: Simple tree rendering test looks solid.Covers the essential checks: header rendering, resource presence, and tree connector characters.
90-132: Good coverage for change summary including replace semantics.Both tests validate that
GetChangeSummarycorrectly counts actions, with the second test confirming that "replace" increments both add and remove counters. This aligns with Terraform's semantics.
188-263: Comprehensive extractReferences test cases.Table-driven tests cover key scenarios: simple references, module-qualified references, prefix handling, filtering of var/local references, and nil expression handling. This directly validates the fix for the module-qualified reference extraction logic flagged in previous reviews.
pkg/terraform/ui/parser.go (3)
15-25: Appropriate buffer sizing for Terraform JSON.The 1MB buffer handles large JSON lines well. Good practice to use scanner.Buffer() to prevent token-too-long errors with complex Terraform plans.
36-59: Iterative approach correctly avoids stack overflow.The loop-based empty line skipping addresses the previous review concern about tail recursion. The raw bytes copy (
append([]byte{}, line...)) properly prevents issues from scanner buffer reuse.
71-166: Clean type dispatch pattern.The switch handles all defined message types with consistent unmarshaling. Unknown types gracefully fall back to BaseMessage. Structure is maintainable for adding new message types.
pkg/terraform/ui/init_model.go (4)
54-71: Well-structured model initialization.Uses theme colors from
pkg/ui/theme/colors.goas required. MiniDot spinner provides subtle visual feedback appropriate for init operations.
111-129: Clean message handling with ANSI stripping.Properly strips ANSI codes to prevent display corruption. The operation tracking and viewport management logic is straightforward and maintainable.
174-181: Rune-aware truncation correctly implemented.Using
runewidth.StringWidthandrunewidth.Truncateproperly handles multi-byte UTF-8 characters. This addresses the previous review concern about byte-based slicing.
217-234: Edge case in formatAction for empty subCommand.The default case at line 229-232 could panic if
m.subCommandis empty, thoughstrings.ToUpper(m.subCommand[:1])on an empty string would cause an index out of range. Thelen > 0check protects this, but returning the empty string seems odd.Consider whether an empty subCommand is a valid state that should be handled differently or logged.
docs/prd/terraform-streaming-ui.md (1)
1-244: Comprehensive PRD documentation.Well-structured documentation covering problem statement, solution design, configuration options, and implementation details. The supported commands list now correctly includes
destroy. Good alignment with the implementation.pkg/terraform/ui/tree.go (5)
56-71: External command execution without timeout.
BuildDependencyTreeusesexec.CommandContextbut relies solely on the passed context for timeout. If the caller doesn't set a deadline,terraform showcould hang indefinitely.Verify that callers always pass a context with appropriate timeout, or consider adding a local deadline.
88-101: Composite action handling looks correct.The logic properly identifies replace operations (len == 2) and single actions. This addresses the previous review about missing composite action detection.
216-260: Module-qualified reference extraction now handles nested modules.The updated logic correctly extracts resource addresses from module-qualified references and applies prefixes appropriately. This addresses the previous critical review about dependency graph corruption.
772-816: WCAG-compliant contrast calculation.Clean implementation of relative luminance formula for accessibility. Using 0.5 threshold provides good contrast for badge text.
818-835: Custom hex parsing uses static error.Good use of
errUtils.ErrParseHexColorfor error wrapping, following the coding guidelines for static errors.pkg/terraform/ui/executor.go (6)
43-59: Appropriate TTY and CI environment checks.The early checks for TTY support and CI environment align with the streaming UI design. DryRun bypass is clean.
68-80: Stderr passes through correctly.Passing stderr directly to
os.Stderrallows terraform warnings and prompts to display normally. This is the right approach for interactive flows.
136-164: Argument building avoids integer overflow.Using
appendfor capacity growth as noted in comments prevents the overflow concerns from previous static analysis hints.
196-212: Planfile cleanup on error path.The temp planfile is properly removed on both error and success paths. Good resource management.
320-354: ShouldUseStreamingUI logic is clear and correct.Decision flow: explicit flag override → config check → CI auto-disable → TTY check → subcommand filter. Note that "refresh" is explicitly disabled due to poor JSON streaming support.
635-677: Silent error handling for terraform output fetch.
fetchAndDisplayOutputssilently ignores errors, which is appropriate here since outputs may not exist yet. The function is used for informational display only.pkg/terraform/ui/types.go (5)
1-9: Clear package documentation.Package comment explains the purpose well: streaming TUI for Terraform operations with Docker-build-style interface.
10-29: Comprehensive message type coverage.Constants cover all Terraform machine-readable UI message types. Including
MessageTypeResourceDriftis forward-looking for drift detection scenarios.
227-242: Trailing periods now present on comments.All inline field comments end with periods, addressing the previous review concern about godot linter compliance.
197-225: ResourceState enum with String() method.Clean implementation with explicit handling for unknown states. The String() method is useful for logging and debugging.
244-275: Phase enum follows same pattern.Consistent with ResourceState implementation. Both enums provide clear string representations.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
pkg/terraform/ui/tree.go (4)
56-71: Add performance tracking and use static errors.Per coding guidelines, public functions should include
defer perf.Track(atmosConfig, "pkg.FuncName")()(usenilif noatmosConfigparam). Additionally, errors at lines 62 and 67 should wrap static errors fromerrors/errors.gorather than using plainfmt.Errorfwith string messages.Based on coding guidelines: "All errors MUST be wrapped using static errors defined in errors/errors.go" and "Add defer perf.Track to all public functions."
🔎 Example pattern
func BuildDependencyTree(ctx context.Context, planfilePath, terraformPath, workingDir, stack, component string) (*DependencyTree, error) { + defer perf.Track(nil, "ui.BuildDependencyTree")() + // Run terraform show -json planfile. cmd := exec.CommandContext(ctx, terraformPath, "show", "-json", planfilePath) cmd.Dir = workingDir output, err := cmd.Output() if err != nil { - return nil, fmt.Errorf("failed to run terraform show: %w", err) + return nil, fmt.Errorf("%w: failed to run terraform show: %w", errUtils.ErrRunTerraformShow, err) } var plan tfjson.Plan if err := json.Unmarshal(output, &plan); err != nil { - return nil, fmt.Errorf("failed to parse plan JSON: %w", err) + return nil, fmt.Errorf("%w: failed to parse plan JSON: %w", errUtils.ErrParsePlanJSON, err) } return buildTreeFromPlan(&plan, stack, component) }Note:
ErrRunTerraformShowandErrParsePlanJSONshould be defined inerrors/errors.go.
280-293: Add performance tracking.
RenderTreeis a public method and should includedefer perf.Track(nil, "ui.DependencyTree.RenderTree")()per coding guidelines.Based on coding guidelines.
595-598: Add performance tracking.
GetChangeSummaryis a public method and should includedefer perf.Track(nil, "ui.DependencyTree.GetChangeSummary")()per coding guidelines.Based on coding guidelines.
728-775: Add performance tracking.
RenderChangeSummaryBadgesis a public function and should includedefer perf.Track(nil, "ui.RenderChangeSummaryBadges")()per coding guidelines.Based on coding guidelines.
📜 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 (3)
pkg/terraform/ui/tree.gopkg/terraform/ui/tree_test.gopkg/ui/theme/icons.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:
pkg/ui/theme/icons.gopkg/terraform/ui/tree_test.gopkg/terraform/ui/tree.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/terraform/ui/tree_test.go
🧠 Learnings (13)
📓 Common learnings
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.
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
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.
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: website/docs/cheatsheets/vendoring.mdx:70-70
Timestamp: 2024-11-12T13:06:56.194Z
Learning: In `atmos vendor pull --everything`, the `--everything` flag uses the TTY for TUI but is not interactive.
📚 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/ui/theme/icons.gopkg/terraform/ui/tree_test.gopkg/terraform/ui/tree.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 : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/terraform/ui/tree_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/terraform/ui/tree_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/terraform/ui/tree_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/terraform/ui/tree_test.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/terraform/ui/tree_test.gopkg/terraform/ui/tree.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/terraform/ui/tree_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 : Use colors from pkg/ui/theme/colors.go for all UI output
Applied to files:
pkg/terraform/ui/tree_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/terraform/ui/tree_test.go
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
pkg/terraform/ui/tree.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/terraform/ui/tree.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:
pkg/terraform/ui/tree.go
🧬 Code graph analysis (1)
pkg/terraform/ui/tree_test.go (1)
pkg/terraform/ui/tree.go (2)
DependencyTree(29-34)TreeNode(37-44)
⏰ 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 (macos)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (2)
pkg/ui/theme/icons.go (1)
14-14: Clean addition of the refresh icon.The new
IconRefreshconstant follows the existing pattern with proper documentation. The "↻" symbol is semantically appropriate for indicating replace/recreate operations, and it's actively used in the streaming UI tree rendering.pkg/terraform/ui/tree_test.go (1)
11-263: Comprehensive test coverage with good structure.The test suite effectively covers the key functionality with table-driven tests and behavior-focused assertions. Tests for
colorizedActionSymbol, tree rendering, change summaries, sorting, and reference extraction all follow the coding guidelines well.Note: Error cases for
BuildDependencyTree(exec failures, JSON parse errors) aren't covered, but since those require mockingexec.CommandContext, they're better suited for integration tests or would require dependency injection.
Non-TTY tests don't make sense for the streaming UI feature since the diagnostic formatting functionality only exists in TTY mode. The TTY tests provide complete coverage for diagnostic streaming. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/ui/theme/colors.go (1)
28-47: Don’t expand the deprecated surface area (and fix comment punctuation).
ColorBlackis added inside the “DEPRECATED” legacy block (Line 30). If this is meant for new UI behavior, prefer introducing/using a theme-aware color/style instead, so new call sites don’t start depending on legacy constants (based on learnings/guidelines about avoiding hardcoded colors and usingpkg/ui/theme/colors.go+ theme-aware styles).- The inline deprecation comment on Line 30 should end with a period to align with
godotexpectations for new comments.Proposed tweak
- ColorBlack = "#000000" // Deprecated: Use theme-aware colors + ColorBlack = "#000000" // Deprecated: Use theme-aware colors.
🤖 Fix all issues with AI agents
In @pkg/terraform/ui/confirm.go:
- Around line 42-66: Add performance tracking to the public function
ConfirmDestroy by calling defer perf.Track(nil) at the top of the function (use
nil because ConfirmDestroy has no atmosConfig), and ensure the perf package is
imported; place the defer as the first statement inside ConfirmDestroy so it
runs even if the function returns early (e.g., on TTY check).
- Around line 15-39: The public function ConfirmApply is missing the required
performance tracking; add a deferred perf.Track call at the start of
ConfirmApply using nil for the atmosConfig (i.e., immediately after entering
ConfirmApply do a defer perf.Track(nil)), and add the perf package to the
imports so the call compiles; keep the rest of ConfirmApply unchanged.
🧹 Nitpick comments (6)
tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_no-name-prod.stdout.golden (1)
23-26: Golden snapshot change: please ensure it was generated via-regenerate-snapshots(not hand-edited).Adding the
"ui": { "enabled": false }block here makes sense for the new UI config, but per repo guidelines fortests/snapshots/**/*, this should come from running the snapshot regeneration flow so output stays consistent across environments and future updates.errors/errors.go (1)
82-82: LGTM! General model type error added.This sentinel provides a more general error for model type assertions beyond spinners. While it's similar to
ErrSpinnerUnexpectedModelTypeabove it, this is appropriate for the broader TUI implementation. If consolidation makes sense later, that can be handled in a separate refactor.pkg/terraform/ui/confirm.go (1)
15-66: Consider extracting shared confirmation logic.Both functions share nearly identical structure, differing only in the prompt message. You could reduce duplication with a helper function.
♻️ Optional refactor to reduce duplication
+// confirm is a helper that prompts the user with a yes/no question. +func confirm(title string) (bool, error) { + // Check TTY availability before showing prompt. + if !term.IsTTYSupportForStdout() || !term.IsTTYSupportForStdin() { + return false, errUtils.ErrStreamingNotSupported + } + + var result bool + theme := uiutils.NewAtmosHuhTheme() + + prompt := huh.NewConfirm(). + Title(title). + Affirmative("Yes"). + Negative("No"). + Value(&result). + WithButtonAlignment(lipgloss.Left). + WithTheme(theme) + + if err := prompt.Run(); err != nil { + if errors.Is(err, huh.ErrUserAborted) { + return false, errUtils.ErrUserAborted + } + return false, err + } + return result, nil +} + func ConfirmApply() (bool, error) { defer perf.Track(nil, "ui.ConfirmApply")() - // Check TTY availability before showing prompt. - if !term.IsTTYSupportForStdout() || !term.IsTTYSupportForStdin() { - return false, errUtils.ErrStreamingNotSupported - } - - var confirm bool - theme := uiutils.NewAtmosHuhTheme() - - prompt := huh.NewConfirm(). - Title("Do you want to apply these changes?"). - Affirmative("Yes"). - Negative("No"). - Value(&confirm). - WithButtonAlignment(lipgloss.Left). - WithTheme(theme) - - if err := prompt.Run(); err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return false, errUtils.ErrUserAborted - } - return false, err - } - return confirm, nil + return confirm("Do you want to apply these changes?") } func ConfirmDestroy() (bool, error) { defer perf.Track(nil, "ui.ConfirmDestroy")() - // Check TTY availability before showing prompt. - if !term.IsTTYSupportForStdout() || !term.IsTTYSupportForStdin() { - return false, errUtils.ErrStreamingNotSupported - } - - var confirm bool - theme := uiutils.NewAtmosHuhTheme() - - prompt := huh.NewConfirm(). - Title("Do you want to destroy these resources?"). - Affirmative("Yes"). - Negative("No"). - Value(&confirm). - WithButtonAlignment(lipgloss.Left). - WithTheme(theme) - - if err := prompt.Run(); err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return false, errUtils.ErrUserAborted - } - return false, err - } - return confirm, nil + return confirm("Do you want to destroy these resources?") }pkg/terraform/ui/resource.go (3)
216-223: Add performance tracking for consistency.
GetDiagnostics()should includedefer perf.Track(nil, "terraform.ui.ResourceTracker.GetDiagnostics")()for consistency with other public methods that perform similar operations (likeGetResources()).As per coding guidelines and repository policy, performance tracking is enforced on all public functions via linting, including methods that perform copying operations.
♻️ Add perf tracking
// GetDiagnostics returns all diagnostic messages. func (rt *ResourceTracker) GetDiagnostics() []*DiagnosticMessage { + defer perf.Track(nil, "terraform.ui.ResourceTracker.GetDiagnostics")() + rt.mu.RLock() defer rt.mu.RUnlock() result := make([]*DiagnosticMessage, len(rt.diagnostics)) copy(result, rt.diagnostics) return result }
282-295: Add performance tracking for consistency.
GetCurrentActivity()should includedefer perf.Track(nil, "terraform.ui.ResourceTracker.GetCurrentActivity")()to maintain consistency with other public methods that perform iteration (e.g.,GetActiveCount(),GetCompletedCount()).As per coding guidelines, performance tracking is enforced on all public functions via linting to maintain consistency and avoid case-by-case judgment calls.
♻️ Add perf tracking
// GetCurrentActivity returns the first in-progress or refreshing resource, if any. // Returns nil if no resource is currently active. func (rt *ResourceTracker) GetCurrentActivity() *ResourceOperation { + defer perf.Track(nil, "terraform.ui.ResourceTracker.GetCurrentActivity")() + rt.mu.RLock() defer rt.mu.RUnlock() for _, addr := range rt.order { if op, ok := rt.resources[addr]; ok { if op.State == ResourceStateInProgress || op.State == ResourceStateRefreshing { opCopy := *op return &opCopy } } } return nil }
298-314: Add performance tracking for consistency.
HasErrors()should includedefer perf.Track(nil, "terraform.ui.ResourceTracker.HasErrors")()for consistency with other public methods that perform iteration (e.g.,GetErrorCount()).As per coding guidelines, performance tracking is enforced on all public functions via linting.
♻️ Add perf tracking
// HasErrors returns true if any resources failed. func (rt *ResourceTracker) HasErrors() bool { + defer perf.Track(nil, "terraform.ui.ResourceTracker.HasErrors")() + rt.mu.RLock() defer rt.mu.RUnlock() for _, op := range rt.resources { if op.State == ResourceStateError { return true } } // Also check for error diagnostics. for _, d := range rt.diagnostics { if d.Diagnostic.Severity == "error" { return true } } return false }
📜 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 (18)
errors/errors.gopkg/terraform/ui/confirm.gopkg/terraform/ui/executor.gopkg/terraform/ui/init_model.gopkg/terraform/ui/model.gopkg/terraform/ui/resource.gopkg/terraform/ui/tree_utils.gopkg/ui/theme/colors.gopkg/ui/theme/converter.gotests/cli_test.gotests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_my-legacy-prod-stack.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_no-name-prod.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.goldentests/snapshots/TestCLICommands_describe_component_with_component_name_(backward_compatibility).stdout.goldentests/snapshots/TestCLICommands_describe_component_with_current_directory_(.).stdout.goldentests/test-cases/diagnostic-streaming.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- pkg/terraform/ui/tree_utils.go
- tests/test-cases/diagnostic-streaming.yaml
- pkg/terraform/ui/init_model.go
- pkg/ui/theme/converter.go
- tests/snapshots/TestCLICommands_describe_component_with_component_name_(backward_compatibility).stdout.golden
- tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden
🧰 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:
pkg/terraform/ui/confirm.gotests/cli_test.goerrors/errors.gopkg/terraform/ui/executor.gopkg/terraform/ui/resource.gopkg/ui/theme/colors.gopkg/terraform/ui/model.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/terraform/ui/confirm.gopkg/terraform/ui/executor.gopkg/terraform/ui/resource.gopkg/ui/theme/colors.gopkg/terraform/ui/model.go
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_component_vpc_-s_my-legacy-prod-stack.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.goldentests/snapshots/TestCLICommands_describe_component_with_current_directory_(.).stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_no-name-prod.stdout.golden
**/*_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 fixturesPrefer unit tests with mocks over integration tests - use interfaces and dependency injection for testability, generate mocks with
go.uber.org/mock/mockgen, use table-driven tests, target >80% coverage
Files:
tests/cli_test.go
🧠 Learnings (46)
📓 Common learnings
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.
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.
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.
📚 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/terraform/ui/confirm.goerrors/errors.gopkg/terraform/ui/executor.gopkg/ui/theme/colors.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/terraform/ui/confirm.gopkg/terraform/ui/executor.go
📚 Learning: 2024-10-21T17:51:07.087Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
Applied to files:
pkg/terraform/ui/confirm.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
pkg/terraform/ui/confirm.goerrors/errors.gopkg/terraform/ui/executor.gopkg/terraform/ui/model.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:
pkg/terraform/ui/confirm.gotests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_my-legacy-prod-stack.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_no-name-prod.stdout.golden
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*.go : Use colors from `pkg/ui/theme/colors.go` for all UI theming in Go code
Applied to files:
pkg/terraform/ui/confirm.gopkg/ui/theme/colors.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 cmd/**/*.go : Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Applied to files:
pkg/terraform/ui/confirm.gopkg/terraform/ui/executor.go
📚 Learning: 2024-11-10T18:37:10.032Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_component_utils.go:354-360
Timestamp: 2024-11-10T18:37:10.032Z
Learning: In the vendoring process, a TTY can exist without being interactive. If the process does not prompt the user, we should not require interactive mode to display the TUI. The `CheckTTYSupport` function should check TTY support on stdout rather than stdin.
Applied to files:
pkg/terraform/ui/confirm.gotests/cli_test.gopkg/terraform/ui/executor.go
📚 Learning: 2024-11-16T17:30:52.893Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 786
File: internal/exec/shell_utils.go:159-162
Timestamp: 2024-11-16T17:30:52.893Z
Learning: For the `atmos terraform shell` command in `internal/exec/shell_utils.go`, input validation for the custom shell prompt is not required, as users will use this as a CLI tool and any issues will impact themselves.
Applied to files:
pkg/terraform/ui/confirm.gopkg/terraform/ui/executor.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:
pkg/terraform/ui/confirm.gotests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_my-legacy-prod-stack.stdout.goldenpkg/terraform/ui/executor.gotests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_no-name-prod.stdout.golden
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
pkg/terraform/ui/confirm.gopkg/terraform/ui/executor.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/terraform/ui/confirm.gopkg/terraform/ui/executor.gopkg/terraform/ui/model.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/terraform/ui/confirm.gopkg/terraform/ui/executor.gopkg/terraform/ui/model.go
📚 Learning: 2024-10-21T17:51:53.976Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:53.976Z
Learning: When `atmos terraform clean --everything` is used without specifying a component and without the `--force` flag, prompt the user for confirmation before deleting all components. Use the `--force` flag to skip the confirmation prompt.
Applied to files:
pkg/terraform/ui/confirm.gotests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_no-name-prod.stdout.golden
📚 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/terraform/ui/confirm.gotests/cli_test.goerrors/errors.gopkg/terraform/ui/executor.gopkg/terraform/ui/resource.gopkg/ui/theme/colors.gopkg/terraform/ui/model.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:
tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_my-legacy-prod-stack.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_no-name-prod.stdout.golden
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_my-legacy-prod-stack.stdout.goldentests/snapshots/TestCLICommands_describe_component_with_current_directory_(.).stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_no-name-prod.stdout.golden
📚 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:
tests/cli_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:
errors/errors.gopkg/terraform/ui/executor.gopkg/terraform/ui/model.go
📚 Learning: 2025-04-26T15:54:10.506Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.
Applied to files:
errors/errors.gopkg/terraform/ui/executor.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/terraform/ui/executor.gopkg/terraform/ui/model.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
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.gopkg/terraform/ui/executor.gopkg/terraform/ui/model.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:
errors/errors.gopkg/terraform/ui/executor.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Use `ui.Write/Writef/Writeln()` and `ui.Success/Error/Warning/Info()` for human messages to stderr in Go commands
Applied to files:
errors/errors.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:
errors/errors.gopkg/terraform/ui/executor.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/terraform/ui/executor.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/terraform/ui/executor.go
📚 Learning: 2024-11-24T19:13:10.287Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:407-416
Timestamp: 2024-11-24T19:13:10.287Z
Learning: In `internal/exec/terraform_clean.go`, when `getStackTerraformStateFolder` returns an error in the `handleCleanSubCommand` function, the error is logged, and the process continues without returning the error.
Applied to files:
pkg/terraform/ui/executor.go
📚 Learning: 2025-10-03T18:02:08.535Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: internal/exec/terraform.go:269-272
Timestamp: 2025-10-03T18:02:08.535Z
Learning: In internal/exec/terraform.go, when auth.TerraformPreHook fails, the error is logged but execution continues. This is a deliberate design choice to allow Terraform commands to proceed even if authentication setup fails, rather than failing fast.
Applied to files:
pkg/terraform/ui/executor.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:
pkg/terraform/ui/executor.gopkg/terraform/ui/resource.gopkg/terraform/ui/model.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:
pkg/terraform/ui/executor.gopkg/terraform/ui/resource.gopkg/terraform/ui/model.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:
pkg/terraform/ui/executor.gopkg/terraform/ui/resource.gopkg/terraform/ui/model.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
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/terraform/ui/executor.gopkg/terraform/ui/resource.gopkg/terraform/ui/model.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:
pkg/terraform/ui/executor.gopkg/terraform/ui/resource.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/terraform/ui/executor.gopkg/terraform/ui/model.go
📚 Learning: 2024-12-03T03:52:02.524Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
Applied to files:
pkg/terraform/ui/executor.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/terraform/ui/executor.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/terraform/ui/executor.gopkg/terraform/ui/model.go
📚 Learning: 2025-02-14T23:12:38.030Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:8-8
Timestamp: 2025-02-14T23:12:38.030Z
Learning: Test snapshots in the Atmos project, particularly for dry run scenarios, may be updated during the development process, and temporary inconsistencies in their content should not be flagged as issues.
Applied to files:
tests/snapshots/TestCLICommands_describe_component_with_current_directory_(.).stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden
📚 Learning: 2025-02-05T20:16:24.036Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: pkg/ui/markdown/renderer.go:75-75
Timestamp: 2025-02-05T20:16:24.036Z
Learning: The Purple constant for markdown rendering is defined in pkg/ui/markdown/colors.go.
Applied to files:
pkg/ui/theme/colors.go
📚 Learning: 2025-02-05T20:16:24.036Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: pkg/ui/markdown/renderer.go:75-75
Timestamp: 2025-02-05T20:16:24.036Z
Learning: The Purple constant is defined in pkg/ui/markdown/colors.go as "#9B51E0" and is part of the base colors used for markdown rendering.
Applied to files:
pkg/ui/theme/colors.go
📚 Learning: 2025-01-25T04:01:58.095Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: internal/exec/docs_generate.go:98-101
Timestamp: 2025-01-25T04:01:58.095Z
Learning: In the `generateSingleReadme` function of the docs generation feature (internal/exec/docs_generate.go), errors from `fetchAndParseYAML` should be logged and skipped rather than causing early returns. This is by design to process all inputs and collect all errors, instead of failing fast on the first error.
Applied to files:
pkg/terraform/ui/model.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/terraform/ui/model.go
📚 Learning: 2025-09-10T21:17:55.273Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden
📚 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:
tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_no-name-prod.stdout.golden
🧬 Code graph analysis (3)
tests/cli_test.go (1)
pkg/logger/log.go (1)
Info(34-36)
pkg/terraform/ui/executor.go (12)
internal/tui/templates/term/term_writer.go (1)
IsTTYSupportForStdout(121-123)errors/errors.go (7)
ErrStreamingNotSupported(94-94)ErrStdoutPipe(47-47)ErrStderrPipe(48-48)ErrCommandStart(49-49)ErrTUIRun(46-46)ErrUnexpectedModelType(82-82)ExitCodeError(791-793)pkg/terminal/terminal.go (3)
Stdin(83-83)Stdout(84-84)Stderr(85-85)pkg/ui/formatter.go (4)
Errorf(237-244)Writef(382-384)Write(338-344)Error(226-233)pkg/terraform/ui/model.go (2)
NewModel(74-108)Model(31-47)pkg/io/global.go (1)
UI(33-33)pkg/terraform/ui/tree_builder.go (1)
BuildDependencyTree(18-35)pkg/terraform/ui/tree_render.go (1)
RenderChangeSummaryBadges(408-457)pkg/terraform/ui/init_model.go (1)
InitModel(29-43)pkg/terraform/ui/confirm.go (2)
ConfirmDestroy(42-66)ConfirmApply(15-39)pkg/terraform/ui/types.go (1)
OutputValue(169-174)pkg/ui/theme/table.go (3)
TableConfig(24-31)TableStyleMinimal(18-18)CreateTable(45-58)
pkg/terraform/ui/resource.go (1)
pkg/terraform/ui/types.go (26)
ResourceOperation(228-242)Phase(245-245)ChangeSummaryMessage(163-166)DiagnosticMessage(148-151)VersionMessage(41-45)OutputsMessage(177-180)PhaseInitializing(249-249)PlannedChangeMessage(67-70)ApplyStartMessage(82-85)ApplyProgressMessage(88-91)ApplyCompleteMessage(94-97)ApplyErroredMessage(100-103)RefreshStartMessage(113-116)RefreshCompleteMessage(119-122)Diagnostic(139-145)PhaseError(254-254)Changes(154-160)PhaseComplete(253-253)PhasePlanning(251-251)ResourceStatePending(202-202)PhaseApplying(252-252)ResourceStateInProgress(204-204)ResourceStateComplete(205-205)ResourceStateError(206-206)PhaseRefreshing(250-250)ResourceStateRefreshing(203-203)
⏰ 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 (10)
tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden (1)
19-20: Snapshot correctly reflects the new UI configuration.The addition of the
uiblock withenabled: falsealigns with the PR's introduction of the Terraform streaming UI feature. The default value ensures backward compatibility by making the feature opt-in.tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden (1)
19-20: Snapshot properly regenerated with new UI config.The added
ui.enabledconfiguration aligns with the PR's Terraform streaming UI feature. The snapshot was correctly regenerated using the-regenerate-snapshotsflag (as noted in commit messages), and the YAML structure is consistent with the rest of the config output.tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_my-legacy-prod-stack.stdout.golden (1)
23-26: Snapshot properly reflects new UI configuration.The addition of the
ui.enabledblock is consistent with the streaming UI feature being added in this PR. The snapshot appears correctly regenerated.tests/snapshots/TestCLICommands_describe_component_with_current_directory_(.).stdout.golden (1)
21-22: LGTM - UI config block added correctly.The snapshot has been properly regenerated to include the new
ui.enabledconfiguration. Consistent with the streaming UI feature implementation.tests/cli_test.go (1)
629-643: Nice improvement - exit code handling now works correctly for TTY tests.The change properly captures and returns the wait error, enabling tests to observe exit codes from PTY execution. This aligns well with the PR's focus on streaming TUI reliability.
The logging at Info level is appropriate for test diagnostics since verbosity is configurable via TestMain (lines 690-697), and the pattern matches existing usage throughout the file.
errors/errors.go (4)
29-29: LGTM! Appropriate error sentinel for hex color parsing.This error sentinel follows Go conventions and is well-placed alongside other parsing errors. Supports the theme color usage guideline.
47-49: LGTM! Well-structured command execution error sentinels.These three sentinels are appropriately grouped and provide clear error classification for the streaming command pipeline. The messages are descriptive and follow Go error conventions.
94-94: LGTM! Appropriate control-flow sentinel for streaming UI.Well-placed in the TTY errors section since streaming UI requires terminal capabilities. The error message clearly indicates an environment constraint, which aligns with the fallback behavior described in the PR.
121-121: LGTM! Terraform output parsing error properly classified.Correctly placed in the recoverable errors section alongside related Terraform output sentinels. The error is appropriately scoped for the streaming JSON parser usage mentioned in the PR context.
pkg/terraform/ui/confirm.go (1)
17-19: TTY checks are correctly implemented.The function properly validates both stdin (for user input) and stdout (for display) TTY support before showing the interactive prompt. This aligns with established patterns.
Based on learnings.
Add performance tracking to public functions per coding guidelines. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/terraform/ui/confirm.go (1)
44-71: ConfirmDestroy follows the same pattern as ConfirmApply.The implementation is correct. The two functions share significant structure—if you'd like to reduce duplication, a helper could extract the common prompt logic, passing the title as a parameter.
♻️ Optional refactor to reduce duplication
You could introduce a shared helper:
func confirmWithPrompt(title string, funcName string) (bool, error) { defer perf.Track(nil, funcName)() if !term.IsTTYSupportForStdout() || !term.IsTTYSupportForStdin() { return false, errUtils.ErrStreamingNotSupported } var confirm bool theme := uiutils.NewAtmosHuhTheme() prompt := huh.NewConfirm(). Title(title). Affirmative("Yes"). Negative("No"). Value(&confirm). WithButtonAlignment(lipgloss.Left). WithTheme(theme) if err := prompt.Run(); err != nil { if errors.Is(err, huh.ErrUserAborted) { return false, errUtils.ErrUserAborted } return false, err } return confirm, nil } func ConfirmApply() (bool, error) { return confirmWithPrompt("Do you want to apply these changes?", "ui.ConfirmApply") } func ConfirmDestroy() (bool, error) { return confirmWithPrompt("Do you want to destroy these resources?", "ui.ConfirmDestroy") }
📜 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 (1)
pkg/terraform/ui/confirm.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/terraform/ui/confirm.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/terraform/ui/confirm.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: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Separate I/O (streams) from UI (formatting) using two-layer architecture - use `pkg/io/` for streams and `pkg/ui/` for formatting
📚 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/terraform/ui/confirm.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/terraform/ui/confirm.go
📚 Learning: 2024-10-21T17:51:07.087Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
Applied to files:
pkg/terraform/ui/confirm.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
pkg/terraform/ui/confirm.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:
pkg/terraform/ui/confirm.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 cmd/**/*.go : Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Applied to files:
pkg/terraform/ui/confirm.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*.go : Use colors from `pkg/ui/theme/colors.go` for all UI theming in Go code
Applied to files:
pkg/terraform/ui/confirm.go
📚 Learning: 2024-11-10T18:37:10.032Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_component_utils.go:354-360
Timestamp: 2024-11-10T18:37:10.032Z
Learning: In the vendoring process, a TTY can exist without being interactive. If the process does not prompt the user, we should not require interactive mode to display the TUI. The `CheckTTYSupport` function should check TTY support on stdout rather than stdin.
Applied to files:
pkg/terraform/ui/confirm.go
📚 Learning: 2024-11-16T17:30:52.893Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 786
File: internal/exec/shell_utils.go:159-162
Timestamp: 2024-11-16T17:30:52.893Z
Learning: For the `atmos terraform shell` command in `internal/exec/shell_utils.go`, input validation for the custom shell prompt is not required, as users will use this as a CLI tool and any issues will impact themselves.
Applied to files:
pkg/terraform/ui/confirm.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:
pkg/terraform/ui/confirm.go
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
pkg/terraform/ui/confirm.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/terraform/ui/confirm.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/terraform/ui/confirm.go
📚 Learning: 2024-10-21T17:51:53.976Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:53.976Z
Learning: When `atmos terraform clean --everything` is used without specifying a component and without the `--force` flag, prompt the user for confirmation before deleting all components. Use the `--force` flag to skip the confirmation prompt.
Applied to files:
pkg/terraform/ui/confirm.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
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/terraform/ui/confirm.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:
pkg/terraform/ui/confirm.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:
pkg/terraform/ui/confirm.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:
pkg/terraform/ui/confirm.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:
pkg/terraform/ui/confirm.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/terraform/ui/confirm.go
🧬 Code graph analysis (1)
pkg/terraform/ui/confirm.go (5)
pkg/perf/perf.go (1)
Track(121-138)internal/tui/templates/term/term_writer.go (2)
IsTTYSupportForStdout(121-123)IsTTYSupportForStdin(133-135)errors/errors.go (2)
ErrStreamingNotSupported(94-94)ErrUserAborted(618-618)internal/tui/utils/utils.go (1)
NewAtmosHuhTheme(166-192)pkg/runner/runner.go (1)
Run(74-107)
⏰ 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 (2)
pkg/terraform/ui/confirm.go (2)
3-13: Import organization looks good.Follows the three-group structure with proper sorting and maintained aliases.
15-42: ConfirmApply implementation is solid.TTY checks, error handling, theming, and perf tracking all follow established patterns correctly.
The tree continuation character (│) for attribute lines was misaligned with the tree connector (├/└) for resource lines by 1 character. This was caused by attribute lines using 6 spaces of indentation while resource lines only used 5 characters before the tree connector. Changed all 5 attribute format strings from 6 to 5 spaces of indentation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/terraform/ui/tree_render.go (2)
68-127: Consider nil safety for attribute changes.The function iterates over
changeswithout checking for nil elements. If the slice contains a nil*AttributeChange, accessingchange.Key,change.Before, etc. would panic.💡 Optional defensive check
for i, change := range changes { + if change == nil { + continue + } _, beforeIsMultiline := getRawStringValue(change.Before, change.Sensitive)
274-291: Diff algorithm collects lines from both sides simultaneously.The current approach increments both
iandjwhen lines differ, which pairs lines by position rather than finding the minimal edit distance. For example, if one line is inserted, all subsequent lines will show as changed.This is likely intentional for simplicity and performance, but worth noting if you ever see unexpected diff output.
If semantic diffing becomes important, consider an LCS-based approach. For now, this positional comparison should work fine for typical Terraform plan output where structure is consistent.
📜 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 (1)
pkg/terraform/ui/tree_render.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/terraform/ui/tree_render.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/terraform/ui/tree_render.go
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Separate I/O (streams) from UI (formatting) using two-layer architecture - use `pkg/io/` for streams and `pkg/ui/` for formatting
📚 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/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
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/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*.go : Use colors from `pkg/ui/theme/colors.go` for all UI theming in Go code
Applied to files:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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/terraform/ui/tree_render.go
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
pkg/terraform/ui/tree_render.go
📚 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:
pkg/terraform/ui/tree_render.go
📚 Learning: 2025-01-17T00:18:57.769Z
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.
Applied to files:
pkg/terraform/ui/tree_render.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/terraform/ui/tree_render.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/terraform/ui/tree_render.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/terraform/ui/tree_render.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). (58)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (windows)
🔇 Additional comments (10)
pkg/terraform/ui/tree_render.go (10)
1-11: LGTM!Package declaration and imports are properly organized with the three-group separation (stdlib, 3rd-party, atmos packages).
15-30: LGTM!Performance tracking is in place, theme colors are properly used, and the rendering logic is clean. Good separation of concerns by delegating to
renderChildren.
32-66: LGTM!Recursive tree rendering logic is sound. Box-drawing character selection and child prefix handling are correct.
129-220: LGTM!The rendering logic cleanly separates multi-line and single-line cases. The tree continuation handling and key style determination are well thought out.
305-341: LGTM!Clean implementation for rendering single-side multiline values. Truncation and styling are handled correctly.
343-372: LGTM!Good use of theme constants for colors. The switch covers all standard Terraform actions with sensible defaults.
374-380: LGTM!Performance tracking in place. Clean delegation to the recursive helper.
382-403: LGTM!Good nil check and the "replace" counting logic (as both add and remove) is sensible and documented.
405-457: LGTM!Performance tracking in place. Badge rendering is clean with proper theme color usage. The conditional badge creation ensures only relevant badges appear.
459-462: LGTM!Simple test helper using theme colors.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @pkg/terraform/ui/tree_render.go:
- Around line 330-333: The truncation code using maxLineWidth can panic for very
small widths; update the guard around the slice so you only subtract 3 when it's
safe: inside the same block that checks len(line) > maxLineWidth, add a
conditional that if maxLineWidth > 3 then do line = line[:maxLineWidth-3] +
"..." else fall back to line = line[:maxLineWidth]; mirror the safe-check
pattern used in renderMultilineDiff and reference the same variables (line,
maxLineWidth) to avoid negative slice indices.
- Around line 236-242: The truncateLine helper can panic or slice incorrectly
when maxLineWidth <= 3; update truncateLine (the anonymous function assigned to
truncateLine) to guard that case by returning either the full line when
maxLineWidth is <= 0 or a safe ellipsis when maxLineWidth <= 3 (e.g., return
"..." or a substring of "..." limited to maxLineWidth), otherwise perform the
existing truncation using line[:maxLineWidth-3] + "...".
📜 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 (1)
pkg/terraform/ui/tree_render.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/terraform/ui/tree_render.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/terraform/ui/tree_render.go
🧠 Learnings (16)
📓 Common learnings
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Separate I/O (streams) from UI (formatting) using two-layer architecture - use `pkg/io/` for streams and `pkg/ui/` for formatting
📚 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:
pkg/terraform/ui/tree_render.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
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/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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/terraform/ui/tree_render.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*.go : Use colors from `pkg/ui/theme/colors.go` for all UI theming in Go code
Applied to files:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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/terraform/ui/tree_render.go
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
pkg/terraform/ui/tree_render.go
📚 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:
pkg/terraform/ui/tree_render.go
📚 Learning: 2025-01-17T00:18:57.769Z
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.
Applied to files:
pkg/terraform/ui/tree_render.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/terraform/ui/tree_render.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/terraform/ui/tree_render.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/terraform/ui/tree_render.go
🔇 Additional comments (9)
pkg/terraform/ui/tree_render.go (9)
1-11: Imports look good.Clean organization following the three-group pattern. All imports are used.
13-30: Solid implementation.Proper perf tracking, theme color usage, and clean delegation pattern. The two-column layout comment is helpful context.
32-66: Clean recursive tree rendering.Box-drawing logic and prefix handling are correct. Nice touch with the fixed-width column layout comment.
343-372: Nice color mapping for actions.Good use of theme colors and the IconActive constant. The space return for no-op/default maintains column alignment.
374-380: Clean summary extraction.Proper perf tracking and concise delegation to the recursive helper.
382-403: Correct counting logic.Good call on having
replacecount as both add and remove - that accurately reflects the destroy+recreate semantics.
459-462: LGTM.Simple factory for testing purposes - perf.Track exception applies per guidelines.
68-127: Well-structured pre-computation for column alignment.The
formattedChangestruct cleanly encapsulates the computed values.getRawStringValueandformatSimpleValueare properly defined intree_utils.goand accessible to this function.
405-457: Badge rendering looks solid.
getContrastTextColorexists inpkg/terraform/ui/tree_utils.goand is correctly called. Comments properly end with periods, theme colors are used appropriately, andperf.Trackis correctly implemented with the blank line. All Go guidelines met.
Implement enhanced tree rendering for Terraform plan output: - Add clean attribute indentation (remove noisy │ bars by default) - Add RenderConfig struct with Compact, ShowAttributeBar, MaxLines options - Add pretty-printed JSON for complex values (maps, arrays) - Add isComplexValue and collapseIfNeeded utilities - Add renderComplexAttributeChange for JSON diff rendering - Add line-by-line semantic diff for multi-line values - Update tests for new function signatures Config options (atmos.yaml): settings.terraform.ui.compact: false # Add blank lines between resources settings.terraform.ui.show_attribute_bar: true # Show ┃ bar for attributes settings.terraform.ui.max_lines: 0 # 0 = show all (default) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add safety check to prevent panic when maxLineWidth <= 3. The slice operation line[:maxWidth-3] could cause a panic if the terminal width is unexpectedly small. Addresses CodeRabbit review comments. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/schema/schema.go (1)
438-455: Solid design with pointer booleans.The
*booltypes forCompactandShowAttributeBarcorrectly enable three-state logic (nil/true/false). Documentation is clear.Consider adding helper methods for cleaner consumption, similar to the pattern used for
StacksInheritat lines 544-558:♻️ Optional helper methods
// IsCompact returns whether compact mode is enabled. // Defaults to true if not explicitly set. func (t *TerraformUI) IsCompact() bool { if t.Compact == nil { return true // Default per comment. } return *t.Compact } // IsShowAttributeBar returns whether the attribute bar is shown. // Defaults to false if not explicitly set. func (t *TerraformUI) IsShowAttributeBar() bool { if t.ShowAttributeBar == nil { return false // Default per comment. } return *t.ShowAttributeBar }pkg/terraform/ui/tree_render.go (1)
290-327: Unusedconfigparameter.The
configparameter is accepted but never used in the function body. Consider removing it if not needed for future expansion, or add a comment indicating planned usage.♻️ Suggested fix
// renderJSONDiff renders a line-by-line diff of JSON content. -func renderJSONDiff(b *strings.Builder, beforeLines, afterLines []string, indent string, - createStyle, deleteStyle lipgloss.Style, config *RenderConfig, -) { +func renderJSONDiff(b *strings.Builder, beforeLines, afterLines []string, indent string, + createStyle, deleteStyle lipgloss.Style, +) {
📜 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)
pkg/schema/schema.gopkg/terraform/ui/tree_render.gopkg/terraform/ui/tree_test.gopkg/terraform/ui/tree_utils.go
✅ Files skipped from review due to trivial changes (1)
- pkg/terraform/ui/tree_utils.go
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/schema/schema.gopkg/terraform/ui/tree_render.gopkg/terraform/ui/tree_test.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/schema/schema.gopkg/terraform/ui/tree_render.gopkg/terraform/ui/tree_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 fixturesPrefer unit tests with mocks over integration tests - use interfaces and dependency injection for testability, generate mocks with
go.uber.org/mock/mockgen, use table-driven tests, target >80% coverage
Files:
pkg/terraform/ui/tree_test.go
🧠 Learnings (27)
📓 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: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Separate I/O (streams) from UI (formatting) using two-layer architecture - use `pkg/io/` for streams and `pkg/ui/` for formatting
📚 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/schema/schema.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/schema/schema.gopkg/terraform/ui/tree_render.gopkg/terraform/ui/tree_test.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/terraform/ui/tree_render.gopkg/terraform/ui/tree_test.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:
pkg/terraform/ui/tree_render.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
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/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*.go : Use colors from `pkg/ui/theme/colors.go` for all UI theming in Go code
Applied to files:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.gopkg/terraform/ui/tree_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:
pkg/terraform/ui/tree_render.go
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
pkg/terraform/ui/tree_render.go
📚 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:
pkg/terraform/ui/tree_render.go
📚 Learning: 2025-01-17T00:18:57.769Z
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.
Applied to files:
pkg/terraform/ui/tree_render.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/terraform/ui/tree_render.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/terraform/ui/tree_render.go
📚 Learning: 2024-11-01T14:45:32.417Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 757
File: cmd/docs.go:52-54
Timestamp: 2024-11-01T14:45:32.417Z
Learning: In `cmd/docs.go`, capping the terminal width at 120 columns is considered acceptable and preferred after testing.
Applied to files:
pkg/terraform/ui/tree_render.go
📚 Learning: 2024-11-01T15:44:12.617Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 757
File: cmd/docs.go:42-59
Timestamp: 2024-11-01T15:44:12.617Z
Learning: In `cmd/docs.go`, when implementing width detection for the `docsCmd` command, it's acceptable to keep the code inline without extracting it into a separate function, as per the user's preference for compact readability and maintainability in Go code.
Applied to files:
pkg/terraform/ui/tree_render.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*.go : Keep files small and focused - under 600 lines with one cmd/impl per file, co-locate tests, never use `//revive:disable:file-length-limit`
Applied to files:
pkg/terraform/ui/tree_render.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:
pkg/terraform/ui/tree_render.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 : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/terraform/ui/tree_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/terraform/ui/tree_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/terraform/ui/tree_test.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests - use interfaces and dependency injection for testability, generate mocks with `go.uber.org/mock/mockgen`, use table-driven tests, target >80% coverage
Applied to files:
pkg/terraform/ui/tree_test.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*.go : All comments must end with periods (enforced by `godot` linter) in Go code
Applied to files:
pkg/terraform/ui/tree_test.go
📚 Learning: 2025-02-19T05:50:35.853Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden:0-0
Timestamp: 2025-02-19T05:50:35.853Z
Learning: Backtick formatting should only be applied to flag descriptions in Go source files, not in golden test files (test snapshots) as they are meant to capture the raw command output.
Applied to files:
pkg/terraform/ui/tree_test.go
🧬 Code graph analysis (3)
pkg/schema/schema.go (1)
pkg/io/global.go (1)
UI(33-33)
pkg/terraform/ui/tree_render.go (4)
pkg/terraform/ui/tree.go (3)
DependencyTree(5-10)TreeNode(13-20)AttributeChange(23-30)pkg/perf/perf.go (1)
Track(121-138)pkg/ui/theme/colors.go (3)
ColorCyan(33-33)ColorGray(31-31)ColorGreen(32-32)pkg/terraform/ui/types.go (1)
Changes(154-160)
pkg/terraform/ui/tree_test.go (2)
pkg/terraform/ui/tree.go (3)
DependencyTree(5-10)TreeNode(13-20)AttributeChange(23-30)pkg/terraform/ui/tree_render.go (1)
RenderChangeSummaryBadges(457-506)
⏰ 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 (27)
pkg/schema/schema.go (2)
434-436: LGTM!The new
UIfield integrates cleanly with the existingTerraformstruct. Struct tags are consistent with the rest of the file.
844-846: LGTM!Good addition for tracking UI state. The
UIFlagExplicitlySetfield enables proper precedence handling (flag > config > default) which aligns with the coding guidelines for configuration precedence.pkg/terraform/ui/tree_render.go (13)
1-11: LGTM!Imports are well-organized following the three-group pattern.
13-35: LGTM!Good delegation pattern and proper use of theme colors. The
perf.Trackis correctly placed.
37-76: LGTM!Clean recursive tree rendering with proper box-drawing characters and nil-safe config handling.
78-86: LGTM!Well-documented configuration struct.
88-241: LGTM!Comprehensive attribute change rendering with proper theme color usage and clean handling of multi-line/complex values.
243-288: LGTM!Clean handling of complex JSON attribute changes.
329-375: LGTM with note.The small terminal width guard (line 339) is a good defensive fix. Similar to
renderJSONDiff, theconfigparameter is unused here.
377-390: LGTM!Clean multi-line rendering with proper truncation safety.
392-421: LGTM!Proper use of theme colors and icons for action visualization.
423-429: LGTM!Clean summary retrieval with proper performance tracking.
431-452: LGTM!Correct recursive counting with proper nil handling. The replace action counting as both add and remove is well-documented.
454-506: LGTM!Clean badge rendering with proper theme colors and contrast text handling.
508-511: LGTM!Useful test helper with proper theme color usage.
pkg/terraform/ui/tree_test.go (12)
1-10: LGTM!Imports are properly organized.
12-33: LGTM!Good table-driven test covering all action types with appropriate handling of ANSI codes.
35-89: LGTM!Good coverage of simple and complex tree rendering scenarios.
91-133: LGTM!Good coverage of change summary counting, including the replace action edge case.
135-187: LGTM!Good edge case coverage for sorting and rendering with proper connector verification.
189-264: LGTM!Comprehensive table-driven tests for reference extraction covering various Terraform patterns.
266-440: LGTM!Thorough coverage of multiline diff rendering with proper verification of marker ordering and grouping.
442-611: LGTM!Comprehensive attribute change rendering tests with good security coverage for sensitive value masking.
613-702: LGTM!Good coverage of Terraform resource change extraction with replacement path handling.
704-827: LGTM!Comprehensive helper function tests with good edge case coverage including invalid input handling.
829-868: LGTM!Good badge rendering tests with smart regex usage to avoid false positives.
870-933: LGTM!Solid countActions tests with nil safety and recursive counting verification.
Resolved merge conflicts: - pkg/ui/formatter.go: Kept both FormatErrorf and Badge/FormatExperimentalBadge - pkg/ui/theme/icons.go: Kept both IconRefresh and IconExperimental - website/src/data/roadmap.js: Kept streaming UI milestone (shipped) Also fixed many lint issues in pkg/terraform/ui/: - Fixed import aliases (logger → log) - Fixed godot comments (sentence formatting) - Fixed gofumpt formatting - Fixed appendAssign issues - Fixed ifElseChain issues (converted to switch) - Fixed nestingReduce issues - Added nolint for bubbletea models and lipgloss.Style - Fixed staticcheck QF1012 (WriteString → fmt.Fprintf) - Added constants for repeated strings/numbers - Fixed early-return patterns Remaining lint issues (44) are structural: - Cyclomatic complexity (requires function decomposition) - Function/file length limits - Argument limits - gocognit cognitive complexity Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
||
| // Add -out flag to args (copy slice to avoid modifying original). | ||
| planOpts := *opts | ||
| planOpts.Args = make([]string, len(opts.Args)+1) |
Check failure
Code scanning / CodeQL
Size computation for allocation may overflow High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 21 days ago
General approach: Add defensive checks before any allocation whose size expression combines user/tainted lengths with arithmetic, ensuring the computed size cannot overflow int. For ExecutePlan, we guard that len(opts.Args) is not so large that len(opts.Args)+1 would overflow. For EvaluateYqExpression, we guard the size of the intermediate YAML string produced from user data before we feed it into yq and re-marshal, putting a reasonable upper bound on processed YAML size.
Concrete fix for ExecutePlan (pkg/terraform/ui/executor.go):
- Import
mathto usemath.MaxInt. - Immediately before
planOpts.Args = make([]string, len(opts.Args)+1), computen := len(opts.Args)and checkif n > math.MaxInt-1(or equivalently>= math.MaxInt). If so, return a descriptive error using the existingerrUtilsfacilities (orfmt.Errorf) explaining that too many arguments were provided. - Then safely allocate with
make([]string, n+1)and useninstead of recalculatinglen(opts.Args).
This keeps existing behavior for all normal inputs and only adds a failure mode for astronomically large argument counts that would be unsafe anyway.
Concrete fix for EvaluateYqExpression (pkg/utils/yq_utils.go):
-
Decide a conservative maximum YAML size to process in this helper. A few megabytes is plenty for CLI expressions; for example, 64 MiB matches the example in the background and is safe for
inton all platforms. -
After
yamlData, err := ConvertToYAML(data), add a check like:const maxYQInputSize = 64 * 1024 * 1024 if len(yamlData) > maxYQInputSize { return nil, fmt.Errorf("EvaluateYqExpression: YAML input too large (%d bytes, max %d)", len(yamlData), maxYQInputSize) }
-
This prevents
yqprocessing and subsequentyaml.Marshalcalls on unbounded, attacker-controlled content that could stress memory or hit implementation limits. It also directly addresses CodeQL’s taint path from user data to YAML marshaling.
No other snippets need code changes to address this specific overflow warning; the size computations in them are handled inside the standard library or third‑party libraries, and our new guards constrain input size before it reaches those layers.
| @@ -6,6 +6,7 @@ | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "math" | ||
| "os" | ||
| "os/exec" | ||
| "path/filepath" | ||
| @@ -288,9 +289,16 @@ | ||
|
|
||
| // Add -out flag to args (copy slice to avoid modifying original). | ||
| planOpts := *opts | ||
| planOpts.Args = make([]string, len(opts.Args)+1) | ||
| n := len(opts.Args) | ||
| // Guard against potential integer overflow when growing the args slice. | ||
| if n > math.MaxInt-1 { | ||
| return errUtils.Build(errUtils.ErrInvalidCommand). | ||
| WithExplanation(fmt.Sprintf("too many terraform arguments (%d) to safely construct plan options", n)). | ||
| Err() | ||
| } | ||
| planOpts.Args = make([]string, n+1) | ||
| copy(planOpts.Args, opts.Args) | ||
| planOpts.Args[len(opts.Args)] = "-out=" + planFile | ||
| planOpts.Args[n] = "-out=" + planFile | ||
|
|
||
| // Run plan with TUI. | ||
| if err := Execute(ctx, &planOpts); err != nil { |
| @@ -61,6 +61,12 @@ | ||
| return nil, fmt.Errorf("EvaluateYqExpression: failed to convert data to YAML: %w", err) | ||
| } | ||
|
|
||
| // Guard against excessively large YAML inputs which could cause allocator or library limits to be hit. | ||
| const maxYQInputSize = 64 * 1024 * 1024 // 64 MiB | ||
| if len(yamlData) > maxYQInputSize { | ||
| return nil, fmt.Errorf("EvaluateYqExpression: YAML input too large (%d bytes, max %d)", len(yamlData), maxYQInputSize) | ||
| } | ||
|
|
||
| pref := yqlib.YamlPreferences{ | ||
| Indent: 2, | ||
| ColorsEnabled: false, |
Update golden snapshot files to reflect changes from the main branch merge: - Add new TerraformUI fields (compact, show_attribute_bar, max_lines) - Update Terraform help text output to match current version - Fix minor formatting alignment in provenance output Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@go.mod`:
- Line 72: The dependency "github.com/hashicorp/terraform-json v0.27.1" in
go.mod may not match your target Terraform CLI release; confirm which Terraform
CLI version you support, test the library against that CLI, and if they differ
update the go.mod entry to the terraform-json tag that corresponds to your
target CLI (or add CI tests that run the supported Terraform CLI version against
terraform-json v0.27.1); ensure automated tests validate plan/json output
parsing for the target CLI and document the chosen pairing in the repo (e.g., in
README or a deps comment).
In `@pkg/terraform/ui/model.go`:
- Around line 124-207: Add perf.Track instrumentation to the exported Bubble Tea
entrypoints: Model.Init, Model.Update, and Model.View. At the start of each
method call perf.Track (using a unique operation name like
"terraform/ui.Model.Init", "terraform/ui.Model.Update",
"terraform/ui.Model.View") and ensure the tracking covers the method execution
(for Update wrap the body so tracking begins on entry and ends on return),
import the perf package, and preserve existing return values and behavior
(including tea.Cmd/tea.Model handling) so instrumentation is purely
observational.
In `@pkg/terraform/ui/parser.go`:
- Around line 23-27: The hardcoded 1MB maxScanTokenSize used in scanner.Buffer
can cause bufio.ErrTooLong on large Terraform JSON lines; change
maxScanTokenSize from a fixed const to a configurable value (e.g., a parameter
to the parser constructor or an env/config var) and raise the default (suggest
8*1024*1024). Also add handling after scanner.Scan(): if scanner.Err() ==
bufio.ErrTooLong, fall back to a different reader strategy (e.g., use a
bufio.Reader and ReadBytes('\n') or io.ReadAll for that large chunk) so
streaming UI doesn't abort; update uses of scanner.Buffer and the variable name
maxScanTokenSize to locate the change.
In `@pkg/terraform/ui/tree_render.go`:
- Around line 102-105: The indent calculation uses byte length for the prefix
causing misalignment with multi-byte box‑drawing runes; in the calculation of
baseIndent replace len(prefix) with lipgloss.Width(prefix) (i.e., baseIndent :=
strings.Repeat(" ", 5+lipgloss.Width(prefix)+4)) to compute display width
correctly and, if lipgloss is not already imported in
pkg/terraform/ui/tree_render.go, add the import for
github.com/charmbracelet/lipgloss.
♻️ Duplicate comments (1)
pkg/terraform/ui/tree_builder.go (1)
194-260: Nested module references lose the resource address.For refs like
module.network.module.vpc.aws_subnet.main.id, the current logic collapses tomodule.network.module.vpc, which won’t match resource nodes and can drop dependencies.🛠️ Proposed fix
- if strings.HasPrefix(ref, "module.") { - parts := strings.Split(ref, ".") - - // Count how many "module" keywords we have. - // For nested modules (e.g., module.network.module.vpc.aws_subnet.main), - // we want to extract only the module path (module.network.module.vpc), - // not the resource within it. - // But for single modules (module.vpc.aws_subnet.main), we want the full resource address. - moduleCount := 0 - lastModuleIdx := -1 - for i := 0; i < len(parts); i++ { - if parts[i] == "module" { - moduleCount++ - lastModuleIdx = i - } - } - - if moduleCount > 1 { - // Nested module - extract only up to the last module.name. - if lastModuleIdx >= 0 && lastModuleIdx+1 < len(parts) { - ref = strings.Join(parts[:lastModuleIdx+2], ".") - } - } else { - // Single module - extract module.name.resource_type.resource_name. - // Minimum for a module reference: module.name (2 parts). - // For a resource within a module: module.name.resource_type.resource_name (4+ parts). - if len(parts) >= 4 { - // Extract the module path and resource address. - // e.g., module.vpc.aws_subnet.main.id -> module path is module.vpc, - // resource is aws_subnet.main. - modulePath := parts[0] + "." + parts[1] - resourceType := parts[2] - resourceName := parts[3] - ref = modulePath + "." + resourceType + "." + resourceName - } else if len(parts) >= 2 { - // Just a module reference (module.name) - keep as-is. - ref = parts[0] + "." + parts[1] - } - } + if strings.HasPrefix(ref, "module.") { + parts := strings.Split(ref, ".") + i := 0 + var moduleParts []string + for i+1 < len(parts) && parts[i] == "module" { + moduleParts = append(moduleParts, parts[i], parts[i+1]) + i += 2 + } + if i >= len(parts) { + ref = strings.Join(moduleParts, ".") + refs = append(refs, ref) + continue + } + if i+1 < len(parts) { + ref = strings.Join(append(moduleParts, parts[i], parts[i+1]), ".") + } else { + ref = strings.Join(moduleParts, ".") + } } else {
🧹 Nitpick comments (1)
pkg/terraform/ui/executor.go (1)
289-294: Static analysis false positive - allocation overflow is theoretical only.The CodeQL warning about
len(opts.Args)+1potentially overflowing is technically valid but practically impossible. CLI arguments are bounded by OS limits (typically thousands, not billions). The current implementation is safe for all realistic use cases.That said, using
appendwould avoid the preallocation entirely and silence the warning:🔧 Optional refactor
- planOpts.Args = make([]string, len(opts.Args)+1) - copy(planOpts.Args, opts.Args) - planOpts.Args[len(opts.Args)] = "-out=" + planFile + planOpts.Args = append(append([]string(nil), opts.Args...), "-out="+planFile)
| github.com/hashicorp/hcl/v2 v2.24.0 | ||
| github.com/hashicorp/terraform-config-inspect v0.0.0-20250828155816-225c06ed5fd9 | ||
| github.com/hashicorp/terraform-exec v0.24.0 | ||
| github.com/hashicorp/terraform-json v0.27.1 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
terraform-json v0.27.1 Terraform CLI compatibility security advisories
💡 Result:
Short answer
- terraform-json v0.27.1 is a released tag of the HashiCorp terraform-json library (it appears in the repo tags). [1]
- terraform-json is a snapshot of Terraform CLI JSON output formats and can lag Terraform CLI changes — the project README recommends upgrading the library when you need newer Terraform JSON fields (i.e., it’s not guaranteed forward‑compatible). [1]
- I could not find any published security advisories specifically targeting terraform-json v0.27.1 (no CVE/advisory entries in the sources searched). Snyk’s package page shows no direct vulnerabilities for the module. [2][3]
Notes / actions
- If you depend on exact Terraform CLI JSON fields, treat terraform-json versions as tied to specific Terraform CLI snapshots and test against the CLI version you use. The repo explicitly warns about forward‑compatibility. [1]
- If you want, I can (a) fetch the exact release date and commit/changelog for v0.27.1, (b) run a broader vulnerability database search (CVE, GitHub Security Advisories, OSS‑Fuzz, NVD), or (c) check compatibility mapping between a specific Terraform CLI version and terraform-json v0.27.1. Which would you like?
Sources
[1] third_party/hashicorp/terraform-json (repo tags & README).
[2] mirror of terraform-json tags showing v0.27.1 details.
[3] Snyk package page for github.com/hashicorp/terraform-json (no direct vulnerabilities reported).
Ensure terraform-json v0.27.1 is tested against your target Terraform CLI version.
The terraform-json library is a snapshot of Terraform CLI JSON formats and is not forward-compatible — it needs to be version-locked to the specific Terraform CLI release you support. No security advisories exist for v0.27.1, but verify this version aligns with your Terraform CLI target and test accordingly.
🤖 Prompt for AI Agents
In `@go.mod` at line 72, The dependency "github.com/hashicorp/terraform-json
v0.27.1" in go.mod may not match your target Terraform CLI release; confirm
which Terraform CLI version you support, test the library against that CLI, and
if they differ update the go.mod entry to the terraform-json tag that
corresponds to your target CLI (or add CI tests that run the supported Terraform
CLI version against terraform-json v0.27.1); ensure automated tests validate
plan/json output parsing for the target CLI and document the chosen pairing in
the repo (e.g., in README or a deps comment).
| // Init initializes the model. | ||
| // | ||
| //nolint:gocritic // bubbletea models must be passed by value | ||
| func (m Model) Init() tea.Cmd { | ||
| return tea.Batch( | ||
| m.spinner.Tick, | ||
| m.listenForMessages(), | ||
| m.tick(), | ||
| ) | ||
| } | ||
|
|
||
| // listenForMessages creates a command that listens for parsed messages. | ||
| func (m *Model) listenForMessages() tea.Cmd { | ||
| return func() tea.Msg { | ||
| result, err := m.parser.Next() | ||
| if err != nil { | ||
| if errors.Is(err, io.EOF) { | ||
| return doneMsg{exitCode: 0, err: nil} | ||
| } | ||
| return doneMsg{exitCode: 1, err: err} | ||
| } | ||
| return messageMsg{result: result} | ||
| } | ||
| } | ||
|
|
||
| // tick creates a periodic tick for updating elapsed time. | ||
| func (m *Model) tick() tea.Cmd { | ||
| return tea.Tick(tickInterval, func(t time.Time) tea.Msg { | ||
| return tickMsg(t) | ||
| }) | ||
| } | ||
|
|
||
| // Update handles messages. | ||
| // | ||
| //nolint:gocritic // bubbletea models must be passed by value | ||
| func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { | ||
| switch msg := msg.(type) { | ||
| case tea.WindowSizeMsg: | ||
| m.width = msg.Width | ||
| m.height = msg.Height | ||
| m.progress.Width = min(progressBarWidth, msg.Width-10) | ||
| return m, nil | ||
|
|
||
| case tea.KeyMsg: | ||
| if msg.String() == "ctrl+c" || msg.String() == "q" { | ||
| m.done = true | ||
| return m, tea.Quit | ||
| } | ||
|
|
||
| case spinner.TickMsg: | ||
| var cmd tea.Cmd | ||
| m.spinner, cmd = m.spinner.Update(msg) | ||
| return m, cmd | ||
|
|
||
| case progress.FrameMsg: | ||
| progressModel, cmd := m.progress.Update(msg) | ||
| m.progress = progressModel.(progress.Model) | ||
| return m, cmd | ||
|
|
||
| case tickMsg: | ||
| // Just trigger a re-render for elapsed time updates. | ||
| return m, m.tick() | ||
|
|
||
| case messageMsg: | ||
| if msg.result.Err == nil && msg.result.Message != nil { | ||
| m.tracker.HandleMessage(msg.result.Message) | ||
| } | ||
| return m, m.listenForMessages() | ||
|
|
||
| case doneMsg: | ||
| m.exitCode = msg.exitCode | ||
| m.err = msg.err | ||
| m.done = true | ||
| return m, tea.Quit | ||
| } | ||
|
|
||
| return m, nil | ||
| } | ||
|
|
||
| // View renders the UI. | ||
| // | ||
| //nolint:gocritic // bubbletea models must be passed by value | ||
| func (m Model) View() string { | ||
| if m.done { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add perf.Track to exported Bubble Tea entrypoints.
Init, Update, and View are public and non-trivial; they should be instrumented per repo guidelines.
Proposed fix
func (m Model) Init() tea.Cmd {
+ defer perf.Track(nil, "terraform.ui.Model.Init")()
+
return tea.Batch(
m.spinner.Tick,
m.listenForMessages(),
m.tick(),
)
}
@@
func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
+ defer perf.Track(nil, "terraform.ui.Model.Update")()
+
switch msg := msg.(type) {
case tea.WindowSizeMsg:
@@
}
@@
func (m Model) View() string {
+ defer perf.Track(nil, "terraform.ui.Model.View")()
+
if m.done {🤖 Prompt for AI Agents
In `@pkg/terraform/ui/model.go` around lines 124 - 207, Add perf.Track
instrumentation to the exported Bubble Tea entrypoints: Model.Init,
Model.Update, and Model.View. At the start of each method call perf.Track (using
a unique operation name like "terraform/ui.Model.Init",
"terraform/ui.Model.Update", "terraform/ui.Model.View") and ensure the tracking
covers the method execution (for Update wrap the body so tracking begins on
entry and ends on return), import the perf package, and preserve existing return
values and behavior (including tea.Cmd/tea.Model handling) so instrumentation is
purely observational.
| scanner := bufio.NewScanner(r) | ||
| // Increase buffer size for large JSON lines. | ||
| const maxScanTokenSize = 1024 * 1024 // 1MB | ||
| buf := make([]byte, maxScanTokenSize) | ||
| scanner.Buffer(buf, maxScanTokenSize) |
There was a problem hiding this comment.
Scan buffer limit may fail on large Terraform JSON lines.
A 1 MB max token can trigger bufio.ErrTooLong for big plans/outputs, which would abort streaming UI. Consider raising the limit or making it configurable to avoid hard failures on large stacks.
Suggested adjustment
- // Increase buffer size for large JSON lines.
- const maxScanTokenSize = 1024 * 1024 // 1MB
- buf := make([]byte, maxScanTokenSize)
- scanner.Buffer(buf, maxScanTokenSize)
+ // Increase buffer size for large JSON lines.
+ // Consider making this configurable via CLI/config if very large plans are common.
+ const maxScanTokenSize = 10 * 1024 * 1024 // 10MB
+ buf := make([]byte, 64*1024)
+ scanner.Buffer(buf, maxScanTokenSize)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scanner := bufio.NewScanner(r) | |
| // Increase buffer size for large JSON lines. | |
| const maxScanTokenSize = 1024 * 1024 // 1MB | |
| buf := make([]byte, maxScanTokenSize) | |
| scanner.Buffer(buf, maxScanTokenSize) | |
| scanner := bufio.NewScanner(r) | |
| // Increase buffer size for large JSON lines. | |
| // Consider making this configurable via CLI/config if very large plans are common. | |
| const maxScanTokenSize = 10 * 1024 * 1024 // 10MB | |
| buf := make([]byte, 64*1024) | |
| scanner.Buffer(buf, maxScanTokenSize) |
🤖 Prompt for AI Agents
In `@pkg/terraform/ui/parser.go` around lines 23 - 27, The hardcoded 1MB
maxScanTokenSize used in scanner.Buffer can cause bufio.ErrTooLong on large
Terraform JSON lines; change maxScanTokenSize from a fixed const to a
configurable value (e.g., a parameter to the parser constructor or an env/config
var) and raise the default (suggest 8*1024*1024). Also add handling after
scanner.Scan(): if scanner.Err() == bufio.ErrTooLong, fall back to a different
reader strategy (e.g., use a bufio.Reader and ReadBytes('\n') or io.ReadAll for
that large chunk) so streaming UI doesn't abort; update uses of scanner.Buffer
and the variable name maxScanTokenSize to locate the change.
| // Calculate base indent for attributes (aligned with tree structure). | ||
| // Base indent: 5 spaces (for " ● ") + prefix length + 4 (for "├── "). | ||
| baseIndent := strings.Repeat(" ", 5+len(prefix)+4) | ||
|
|
There was a problem hiding this comment.
Use display width for prefix indentation.
Line 104 uses len(prefix) which counts bytes; box‑drawing runes are multi‑byte, so attributes can drift right. Use lipgloss.Width(prefix) for correct alignment.
Proposed fix
- baseIndent := strings.Repeat(" ", 5+len(prefix)+4)
+ baseIndent := strings.Repeat(" ", 5+lipgloss.Width(prefix)+4)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Calculate base indent for attributes (aligned with tree structure). | |
| // Base indent: 5 spaces (for " ● ") + prefix length + 4 (for "├── "). | |
| baseIndent := strings.Repeat(" ", 5+len(prefix)+4) | |
| // Calculate base indent for attributes (aligned with tree structure). | |
| // Base indent: 5 spaces (for " ● ") + prefix length + 4 (for "├── "). | |
| baseIndent := strings.Repeat(" ", 5+lipgloss.Width(prefix)+4) | |
🤖 Prompt for AI Agents
In `@pkg/terraform/ui/tree_render.go` around lines 102 - 105, The indent
calculation uses byte length for the prefix causing misalignment with multi-byte
box‑drawing runes; in the calculation of baseIndent replace len(prefix) with
lipgloss.Width(prefix) (i.e., baseIndent := strings.Repeat(" ",
5+lipgloss.Width(prefix)+4)) to compute display width correctly and, if lipgloss
is not already imported in pkg/terraform/ui/tree_render.go, add the import for
github.com/charmbracelet/lipgloss.
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
1 participant
what
terraform plan,apply,destroy, andrefreshcommandswhy
references
Related to terraform CLI improvements and TUI enhancements in Atmos.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes / Tweaks
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.