feat: Implement configurable output for list commands #1798
+19,848
−1,201
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ipeline - Implement reusable renderer pipeline (filter, sort, column, format, output) - Create 5 pkg/list packages with >88% test coverage each - Add extraction functions for components, stacks, vendor - Update Priority Tier 1 commands: components (DEV-2805), stacks, vendor (DEV-2806) - Add schema support for List config in Stacks and Components - Create 17 standardized flag wrappers using With* naming convention - Enable universal filtering, sorting, column selection, and format options 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
|
Warning Rate limit exceeded@osterman has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRefactors the list subsystem into a pipeline-driven renderer: adds composable flag wrappers and NewListParser, column templates, filters, sorters, extractors, Renderer and Output manager, provenance-aware import resolution and tree renderers, schema/config additions, many CLI adapters, tests, docs, and tooling for CLAUDE/agent size checks. Changes
Sequence Diagram(s)%% Primary render pipeline (high-level)
sequenceDiagram
participant CLI as CLI
participant Parser as NewListParser
participant Cmd as Command.RunE
participant Extract as Extractor
participant Filter as FilterChain
participant Col as ColumnSelector
participant Sort as MultiSorter
participant Renderer as Renderer
participant Output as OutputManager
CLI->>Parser: parse flags (With*Flag)
Parser-->>CLI: options struct
CLI->>Cmd: invoke RunE with options
Cmd->>Extract: fetch domain data
Extract-->>Cmd: []map[string]any
Cmd->>Filter: build filters from options
Cmd->>Col: resolve columns (flag > config > defaults)
Cmd->>Sort: build sorters from spec
Cmd->>Renderer: New(filters, selector, sorters, format, delimiter)
Cmd->>Renderer: Render(data)
Renderer->>Filter: Apply(data)
Filter-->>Renderer: filtered data
Renderer->>Col: Extract(headers, rows)
Renderer->>Sort: Sort(rows, headers)
Renderer->>Renderer: formatTable(headers, rows)
Renderer->>Output: Write(formatted)
%% Provenance import-resolution flow
sequenceDiagram
participant StackProc as ProcessYAMLConfigFileWithContext
participant Store as MergeContextStore
participant Cache as GetAllMergeContexts
participant Resolver as ResolveImportTreeFromProvenance
participant TreeFmt as RenderStacksTree/RenderInstancesTree
StackProc->>Store: create and persist MergeContext (with ImportChain)
Store-->>Cache: available contexts
Resolver->>Cache: GetAllMergeContexts()
Cache-->>Resolver: contexts w/ import chains
Resolver->>Resolver: build per-stack import trees (detect circular)
Resolver-->>TreeFmt: trees for rendering
TreeFmt->>TreeFmt: render styled tree (spacers, colors)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Migrate workflows, values, and instances commands to use the new renderer pipeline and flag wrapper pattern for consistent flag handling. ## Changes ### cmd/list/workflows.go - Migrate to full renderer pipeline (filter → column → sort → format) - Replace custom flag parsing with NewListParser flag wrappers - Add buildWorkflowFilters(), getWorkflowColumns(), buildWorkflowSorters() - Update return type from string to error - Add support for --columns and --sort flags - Maintain backward compatibility with --file filter - Use static errors from errors/errors.go ### pkg/list/extract_workflows.go (NEW) - Create ExtractWorkflows() to transform workflow manifests into []map[string]any - Extract workflow data with file, workflow name, description, and step count - Support file filtering via --file flag - Handle workflow directory traversal and YAML parsing - Use static errors (ErrParseFile, ErrWorkflowDirectoryDoesNotExist) ### pkg/list/extract_workflows_test.go (NEW) - Add comprehensive tests for workflow extraction - Test empty workflows, single workflow, multiple workflows - Test nil workflows handling ### cmd/list/values.go - Replace newCommonListParser with NewListParser using flag wrappers - Use WithFormatFlag, WithDelimiterFlag, WithStackFlag, WithQueryFlag - Use WithMaxColumnsFlag, WithAbstractFlag, WithProcessTemplatesFlag - Use WithProcessFunctionsFlag - Maintain existing FilterAndListValues logic (already using reusables) - Update both valuesCmd and varsCmd parsers ### cmd/list/instances.go - Replace newCommonListParser with NewListParser using flag wrappers - Use WithFormatFlag, WithDelimiterFlag, WithMaxColumnsFlag - Use WithStackFlag, WithQueryFlag, WithUploadFlag - Maintain existing ExecuteListInstancesCmd logic ## Testing All tests pass: - go test ./cmd/list/... ./pkg/list/... ✓ - All flag wrapper tests pass ✓ - All extraction tests pass ✓ ## Impact - Workflows command now supports universal --columns and --sort flags - Values and instances commands use consistent flag wrapper pattern - All Tier 2 commands follow same architectural pattern as Tier 1 - Maintains backward compatibility with existing flags 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fix critical bug where empty format string caused "unsupported format:" error when running list commands without explicit --format flag. ## Changes ### pkg/list/renderer/renderer.go - Add default handling in formatTable() to use table format when empty - Prevents "unsupported format:" error on commands like `atmos list stacks` ### pkg/list/renderer/renderer_integration_test.go (NEW) - Add comprehensive integration tests for all format types - Test empty format string (should default to table) - Test invalid format string (should error) - Test rendering with sorting - Test rendering with empty data - All tests initialize I/O context properly ## Testing - All renderer tests pass (91.8% coverage) - Fixed user-reported issue: `atmos list stacks` now works - Test all formats: table, json, yaml, csv, tsv, empty ## Impact - Fixes critical regression where basic list commands failed - Empty format now correctly defaults to table format - Maintains backward compatibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Replace thick bordered table with clean header-only underline - Use simple local styles (bold header, plain cells) like version list - Change border to gray (color 8) for consistency - Remove unused theme import This gives list commands the same polished look as atmos version list.
…support - Add ExtractInstances() to convert schema.Instance to renderer format - Add getInstanceColumns() to read column config from atmos.yaml - Update ExecuteListInstancesCmd to use renderer pipeline - Remove obsolete formatInstances() function and related tests - Add custom column configuration to quick-start-advanced example - Support all output formats (table, json, yaml, csv, tsv) Custom columns configured in atmos.yaml (components.list.columns): - Component, Stack, Tenant, Environment, Stage, Region All existing list instances tests pass with renderer migration.
- Move Component column to the end for better readability - Add Description column from metadata.description field - Add description metadata to vpc and vpc-flow-logs-bucket components Column order: Stack → Tenant → Environment → Stage → Region → Description → Component This makes the output more readable by grouping organizational fields (tenant, environment, stage, region) before the descriptive and component fields.
…data to renderer pipeline This change fixes an anti-pattern where list commands were using log.Info() for user-facing messages instead of proper UI output methods. It also migrates the list metadata command to use the renderer pipeline for consistent table output with customizable columns. ## Changes ### Anti-pattern fixes - **cmd/list/settings.go**: Replace `log.Info()` with `ui.Info()` for "No settings found" messages - **cmd/list/values.go**: Replace `log.Info()` with `ui.Info()` for "No vars/values found" messages - **pkg/list/list_metadata.go**: Remove `log.Error()` calls; errors are now properly returned - **cmd/list/settings_test.go**: Update tests for renamed function `displayNoSettingsFoundMessage()` - **cmd/list/values_test.go**: Update tests for renamed function `displayNoValuesFoundMessage()` ### List metadata migration to renderer pipeline - **cmd/list/metadata.go**: Migrate to renderer pipeline (91 lines, down from 161) - **cmd/list/metadata_test.go**: Delete obsolete tests for old flag structure - **pkg/list/extract_metadata.go**: Create metadata extractor for renderer - **pkg/list/extract_metadata_test.go**: Add comprehensive tests (5 test cases) - **pkg/list/list_metadata.go**: Implement renderer integration with default columns ### Documentation updates - **website/docs/cli/commands/list/list-metadata.mdx**: Complete rewrite for table view with column configuration examples - **website/docs/cli/commands/list/list-instances.mdx**: New comprehensive documentation - **website/docs/cli/commands/list/list-vendor.mdx**: New comprehensive documentation - **website/docs/cli/commands/list/list-components.mdx**: Enhanced with column examples - **website/docs/cli/commands/list/list-stacks.mdx**: Add column configuration - **website/docs/cli/commands/list/list-workflows.mdx**: Fix duplicate sections - **website/blog/2025-11-17-customizable-list-command-output.mdx**: Feature announcement blog post ### Breaking changes for list metadata - Removed `--query` flag (use `--filter` with YQ syntax instead) - Removed `--max-columns` flag (use renderer table formatting) - Removed `--delimiter` flag (renderer handles formatting) - Removed positional component argument (use `--filter` for filtering) ## Resolution Fixes issue where `atmos list metadata` displayed "INFO No metadata found" instead of proper UI output. Now uses `ui.Info()` for user-facing messages and renderer pipeline for consistent table formatting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
Apply theme-aware color styling to table cells based on content type:
- Booleans: Green (Success) for 'true', Red (Error) for 'false'
- Numbers: Cyan (Info) for numeric values
- Placeholders: Darker gray (Muted) for {...} and [...] placeholders
- Default text: No color styling (uses base table cell style)
Implementation details:
- Add detectContentType() to identify cell content (boolean, number, placeholder)
- Add getCellStyle() to apply appropriate theme style based on content type
- Update CreateStyledTable() StyleFunc to apply semantic styling
- Use theme.GetCurrentStyles() for theme-aware color consistency
- Add regex pattern matching for placeholder detection ({...}, [...])
- Pass baseStyle by pointer to avoid copying heavy struct (552 bytes)
This enhances readability by visually distinguishing different data types
in list command table output (instances, metadata, workflows, etc.)
added 2 commits
November 17, 2025 08:28
Tables now expand arrays of scalar values (strings, numbers, booleans)
into multi-row cells with one value per line, making array data readable
without needing to expand placeholders.
Changes:
- Add tryExpandScalarArray() to detect and expand scalar arrays
- Arrays with non-scalar elements still show placeholder format
- Add joinItems() helper to join array items with newlines
- Respect MaxColumnWidth for individual array items
- Maps continue to show {...} (N keys) placeholder format
Benefits:
- availability_zones: us-east-1a, us-east-1b, us-east-1c (multi-line)
- Complex arrays: [...] (N items) (placeholder for non-scalar)
- Improves readability for common scalar array fields
Tests:
- Add comprehensive tests for scalar array expansion
- Test mixed types, nested arrays, empty arrays
- Verify placeholder fallback for complex structures
Replace byte-based len() with lipgloss.Width() for proper handling of: - ANSI escape codes (colors, styles) - Multi-byte Unicode characters - Multi-line cell content (expanded arrays) Changes: - Replace len() with lipgloss.Width() in width calculations - Add getMaxLineWidth() to handle multi-line cells correctly - Add splitLines() helper for line-by-line width calculation - Update calculateMaxKeyWidth() to use visual width - Update calculateStackColumnWidth() to use visual width - Update getMaxValueWidth() to find widest line in multi-line values Benefits: - Tables now size columns based on actual visual width - Colored text no longer causes incorrect column sizing - Multi-line array expansions properly measured - More efficient use of terminal space Tests: - Add tests for getMaxLineWidth with ANSI codes - Add tests for splitLines with various inputs - Verify ANSI-aware width calculation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
cmd/list/vendor.go (1)
145-171:--columnsfor vendor shares the same stub implementation
getVendorColumnsalso callsparseColumnsFlag(columnsFlag)when--columnsis set, but that helper is currently a TODO that always returns default columns. So for vendor:
--columns/ATMOS_LIST_COLUMNSdo not actually influence the output today.Once
parseColumnsFlagis implemented for components, it would be good to hook vendor into the same real implementation so behavior is consistent.
🧹 Nitpick comments (20)
CLAUDE.md (1)
39-74: Good addition - consider fixing the markdownlint issue.This section provides clear guidance on when to consult specialized agents. The examples and rationale are helpful.
One minor issue: The fenced code block at line 57 is missing a language identifier. Consider adding one for proper syntax highlighting:
-``` +```text Task( subagent_type: "flag-handler", prompt: "Help design named wrapper functions for list command flags" )</blockquote></details> <details> <summary>website/docs/cli/commands/list/list-vendor.mdx (1)</summary><blockquote> `24-37`: **Minor style nit in description paragraph** The “Target destination path” / “Vendor configuration file” bullets read a bit abruptly after “information about:”. If you care about polish, consider making the last bullet a full phrase like “Vendor configuration file path.” or similar, but the content is otherwise clear. </blockquote></details> <details> <summary>cmd/list/instances.go (1)</summary><blockquote> `15-26`: **InstancesOptions is built but not used downstream** `InstancesOptions` (including the new `Filter` and `Sort` fields) is populated in `RunE` and passed into `executeListInstancesCmd`, but `executeListInstancesCmd` ignores `opts` and just calls `list.ExecuteListInstancesCmd(&configAndStacksInfo, cmd, args)`. If the list package is still reading values directly from Cobra/Viper, this options struct isn’t buying you anything right now and may confuse future readers. I’d either: - Wire `opts` through into `list.ExecuteListInstancesCmd` (if you’re moving instances onto the same options-based renderer pipeline as other list commands), or - Drop `InstancesOptions` / the `opts` parameter for now and reintroduce it when instances actually consume these fields. Also applies to: 47-57, 85-95 </blockquote></details> <details> <summary>cmd/list/settings.go (1)</summary><blockquote> `116-123`: **Consider handling ui.Info errors.** The error returns from `ui.Info` are discarded. While this may be acceptable for informational messages, consider whether these errors should be logged or returned, especially if UI output failures could impact user experience. ```diff func displayNoSettingsFoundMessage(componentFilter string) { if componentFilter != "" { - _ = ui.Info("No settings found for component: " + componentFilter) + if err := ui.Info("No settings found for component: " + componentFilter); err != nil { + log.Debug("Failed to display no settings message", "error", err) + } } else { - _ = ui.Info("No settings found") + if err := ui.Info("No settings found"); err != nil { + log.Debug("Failed to display no settings message", "error", err) + } } }pkg/list/extract_metadata.go (1)
34-50: Simplify string concatenation with strings.Join.The manual string building for
inheritscan be streamlined usingstrings.Join, making the code more concise and idiomatic.if val, ok := instance.Metadata["inherits"].([]interface{}); ok { - // Convert []interface{} to comma-separated string. inheritsSlice := make([]string, 0, len(val)) for _, v := range val { if str, ok := v.(string); ok { inheritsSlice = append(inheritsSlice, str) } } - if len(inheritsSlice) > 0 { - for i, s := range inheritsSlice { - if i > 0 { - inherits += ", " - } - inherits += s - } - } + inherits = strings.Join(inheritsSlice, ", ") }Don't forget to import
stringsif not already imported.pkg/list/list_metadata.go (1)
48-85: Consider adding filter and sort support.The renderer is created with
nilfilters and sorters (line 77), but other list commands support filtering and sorting via CLI flags. Consider whether metadata listing should also support these features for consistency.If filtering and sorting are needed, you could add flags and build filters/sorters similar to other list commands:
func ExecuteListMetadataCmd(info *schema.ConfigAndStacksInfo, cmd *cobra.Command, args []string) error { // Initialize CLI config. atmosConfig, err := cfg.InitCliConfig(*info, true) if err != nil { return errors.Join(errUtils.ErrFailedToInitConfig, err) } - // Get flags. + // Get flags formatFlag, _ := cmd.Flags().GetString("format") + filterFlag, _ := cmd.Flags().GetString("filter") + sortFlag, _ := cmd.Flags().GetString("sort") // Process instances (same as list instances, but we'll extract metadata). instances, err := processInstances(&atmosConfig) if err != nil { return errors.Join(errUtils.ErrProcessInstances, err) } // Extract metadata into renderer-compatible format. data := ExtractMetadata(instances) // Get column configuration. columns := getMetadataColumns(&atmosConfig) + + // Build filters and sorters + filters := buildMetadataFilters(filterFlag) + sorters := buildMetadataSorters(sortFlag) // Create column selector. selector, err := column.NewSelector(columns, column.BuildColumnFuncMap()) if err != nil { return fmt.Errorf("failed to create column selector: %w", err) } // Create renderer. - r := renderer.New(nil, selector, nil, format.Format(formatFlag)) + r := renderer.New(filters, selector, sorters, format.Format(formatFlag)) // Render output. if err := r.Render(data); err != nil { return fmt.Errorf("failed to render metadata: %w", err) } return nil }cmd/list/flag_wrappers_test.go (1)
377-414: Tighten env var binding test or drop unusedenvVarName
TestFlagEnvironmentVariableBindingcurrently only asserts thatBindToVipersucceeds; theenvVarNamefield is never used, so the test doesn’t actually confirm the binding name.Either:
- Use
envVarName(e.g.,t.Setenv(tt.envVarName, "x"), callBindToViper, then assertv.GetString(tt.flagName) == "x"), or- Remove
envVarNamefrom the table to avoid implying stronger coverage than is present.pkg/list/extract_stacks.go (1)
9-73: Consider whether deterministic stack ordering is neededBoth extractors iterate over
stacksMapdirectly, so the output order is whatever Go’s map iteration yields. If the downstream renderer doesn’t always apply an explicit sort (e.g., default sort bystack), CLI output may appear in a non‑deterministic order between runs.If you want stable default ordering, consider sorting the stack names (or ensure callers always run the sort pipeline on these results). Otherwise, the current implementation is straightforward and matches the tests.
pkg/list/renderer/renderer_test.go (1)
38-237: Consider factoring I/O initialization into a helperMost tests duplicate the
NewContext/InitWriter/InitFormattersetup. Extracting a small helper likeinitTestRendererIO(t)would cut repetition and make future tweaks to the IO/UI wiring cheaper.pkg/list/extract_components.go (1)
9-17: Unused exportedComponentDatastruct
ComponentDataisn’t used in this file. If there’s no external consumer yet, consider either wiring it into the API (e.g., returning strongly‑typed slices instead of[]map[string]any) or dropping it for now to avoid a “blessed but unused” public type.pkg/list/extract_components_test.go (1)
11-245: Nice coverage; consider assertingcomponent_typesemantics explicitlyThe tests do a good job exercising success and failure paths. To lock in the intended meaning of metadata fields, you might add a couple of assertions that:
- For an abstract component (like
ingressinTestExtractComponents),comp["component_type"] == "abstract".- For a normal TF component,
comp["type"] == "terraform"andcomp["component_type"] == "real".That would make future refactors around
.typevs.component_typesafer.cmd/list/stacks.go (1)
29-31: Columns flag type vs parsing helper may not line up
StacksOptions.Columnsis astringpopulated viav.GetString("columns"), butWithColumnsFlagregisterscolumnsas aStringSliceflag. Depending on howStandardParserbinds to Viper,GetString("columns")may see something like"[stack,component]"rather than the rawstack,componentstring thatparseColumnsFlagexpects.If the intent is “comma‑separated single value”, you might either:
- Change the flag to a simple string flag; or
- Keep it as a slice and update
StacksOptions+parseColumnsFlagto work with[]string.Worth a quick manual check of
--columns stack,componentand the env var form to confirm the value that reachesparseColumnsFlag.Also applies to: 150-154
website/docs/cli/commands/list/list-stacks.mdx (1)
119-142: Clarify that the listed template functions are a subset of what’s availableThe “Available functions” list here is a subset of what
BuildColumnFuncMapexposes (pad,get,getOr,has,split,toInt,toBool, etc. are also supported). To avoid confusing folks who read the code, consider either:
- Renaming this to “Common functions” and noting it’s not exhaustive, or
- Expanding the list to match the actual func map.
pkg/list/renderer/renderer.go (1)
151-159: Use a CSV/TSV writer to handle escaping correctly
formatDelimitedcurrently does a rawstrings.Joinfor CSV/TSV. That means fields containing the delimiter, quotes, or newlines won’t be escaped and can break consumers.Consider switching to Go’s
encoding/csv(with the delimiter configurable for TSV) so quoting and escaping are handled correctly, e.g., by writing into abytes.Bufferand returning the buffer string.cmd/list/components.go (1)
142-177: Revisit interaction betweentypeandabstractfilters
buildComponentFilterscurrently combines:
Typeflag →filter.NewColumnFilter("type", opts.Type)(default"real"fromWithTypeFlag), andAbstractflag → when!opts.Abstract, always addsfilter.NewColumnFilter("component_type", "real").This can make it hard to express some combinations:
- With no flags you filter on both
type=realandcomponent_type=real.- With
--abstractbut no explicit--type, you still filtertype=real, so abstract-only components may never show.- With
--type=allbut--abstractomitted, you still filtercomponent_typetoreal, so “all” doesn’t actually include abstract.Might be worth clarifying which field (
typevscomponent_type) is the canonical “real/abstract/all” dimension and simplifying defaults around that (e.g., letTypedefault to""and derive the “real-only by default” behavior solely fromAbstract).cmd/list/vendor.go (2)
130-143: Confirm intent of using--stackto glob on thecomponentfield
buildVendorFilterstreats the--stackflag as a glob on the"component"field:globFilter, err := filter.NewGlobFilter("component", opts.Stack)Given
WithStackFlag’s help text (“Filter by stack pattern”) this is a bit surprising. If vendor rows don’t have astackfield and the goal is to filter by component name, it might be clearer to either:
- Introduce a dedicated
--componentfilter for vendor, or- Rename/update the
--stackhelp text for this command.At minimum, worth double-checking that this aligns with how users expect to filter vendor entries.
185-256: Unused home-dir obfuscation helpers after renderer refactorAfter switching vendor listing to the renderer pipeline,
obfuscateHomeDirInOutput,shouldReplaceHomeDir, andisPathCharare no longer used in this file.If the renderer-based output is intentionally allowed to show absolute paths now, these helpers can probably be removed. If you still want to avoid leaking
$HOMEin vendor paths, you may want to integrate this behavior into the data extraction step or the renderer instead.pkg/list/sort/sort.go (1)
110-133: Numeric and multi-column sort behavior is good; tighten comments around edge casesThe type-aware sorting and stable multi-column precedence look solid. Two small nits:
compareNumeric’s comment says “Non-numeric values sort last”, but when both values are non-numeric you fall back tostrings.Compare(a, b). That’s fine behavior, just worth reflecting in the comment so future readers don’t assume “always last”.- In
Sorter.Sort, rows wherecolIdx >= len(row)are effectively treated as “equal” to everything else (you always returnfalsefrom the comparator). That’s a reasonable choice; consider documenting that missing-column rows preserve original order and don’t participate in the sort.No functional changes required; this is just about making the behavior explicit.
Also applies to: 161-185
pkg/list/filter/filter.go (1)
39-86: Filter primitives and chaining look solidThe glob/column/boolean filters and the Chain combinator are straightforward and type-safe, with good validation and error messages. The permissive string handling in
BoolFilter(treating non-true strings as false) is fine for the intendedenabled/lockeduse cases; if you later apply it to arbitrary fields you might want stricter validation, but nothing blocking here.Nice foundation for composing filters into the renderer pipeline.
Also applies to: 119-160, 167-178
pkg/list/column/column.go (1)
27-47: TemplateContext struct is unused; current implementation relies on a generic map-based contextRight now, templates are evaluated against the
map[string]anyproduced bybuildTemplateContext, and theTemplateContextstruct isn’t used anywhere. That’s not wrong, but it’s a bit misleading for readers who might assume templates see a strongly-typed context with the JSON-tagged field names.You can either:
- Remove
TemplateContextuntil you actually switch to using it, or- Update
buildTemplateContextto construct and return aTemplateContext(plus maybe embedRawseparately) so the declared API matches the real context shape.Low priority, but it’ll reduce cognitive overhead for anyone extending the column templating later.
Also applies to: 190-272
added 2 commits
November 17, 2025 08:35
Tables now expand maps with scalar values (like tags) into readable
multi-line format with one key-value pair per line.
Before:
tags {...} (7 keys)
After:
tags Environment: production
Name: vpc
Team: platform
...
Implementation:
- Add tryExpandScalarMap() to detect and expand scalar maps
- Sort keys alphabetically for consistent ordering
- Format as 'key: value' pairs, one per line
- Maps with non-scalar values still show {...} (N keys) placeholder
- Shares joinItems() helper with array expansion
Behavior:
- Scalar maps (string, number, bool values) → Expanded multi-line
- Complex maps (nested objects, arrays) → Placeholder {...} (N keys)
- Empty maps → Empty string
- Keys sorted alphabetically for consistency
Benefits:
- Tags and metadata now readable without expanding
- Consistent with array expansion behavior
- Maintains compact placeholder for complex structures
Tests:
- Add comprehensive tests for scalar map expansion
- Test mixed scalar types, empty maps
- Verify placeholder fallback for nested structures
- Update existing tests to match new behavior
Prevent scalar arrays and maps from being expanded if their items
exceed a reasonable width threshold. This avoids creating impossibly
wide tables that trigger "table too wide" errors.
Problem:
- Expanding arrays/maps with long values caused tables to exceed terminal width
- Users saw: "Error: the table is too wide to display properly (width: 418 > 185)"
- No fallback to compact placeholder format for wide values
Solution:
- Track maximum item width during expansion
- Reject expansion if any item exceeds maxReasonableItemWidth (20 chars)
- Fall back to compact placeholder format: [...] (N items) or {...} (N keys)
- Threshold set to ~1/3 of MaxColumnWidth for reasonable table display
Changes:
- Add width tracking to tryExpandScalarArray()
- Add width tracking to tryExpandScalarMap()
- Use lipgloss.Width() for ANSI-aware measurement
- Set maxReasonableItemWidth constant to 20 characters
Benefits:
- Tables stay within terminal width limits
- Short values still expand (availability_zones, tags with short values)
- Long values gracefully degrade to placeholders
- No more "table too wide" errors from expansion
Tests:
- Add test for arrays with very long strings
- Add test for maps with very long values
- Verify both fall back to placeholder format
- Add renderInlineMarkdown() function to render markdown in table cells - Uses glamour library with theme-aware styling for terminals - Automatically detects color support and degrades gracefully - Collapses multi-line markdown into single line for table cells - Applies to Description column specifically when present in headers - Preserves inline markdown (bold, italic, code) while removing blocks - Includes comprehensive tests for markdown rendering edge cases Resolves markdown rendering for description fields in metadata tables.
…data
**Enhancement 1: Style <no value> in Dark Gray**
- Add contentTypeNoValue enum to cell content type detection
- Update detectContentType() to identify <no value> from Go templates
- Apply styles.Muted (dark gray) styling to <no value> cells
- Consistent with placeholder styling ({...}, [...])
- Theme-aware and zero-config
**Enhancement 2: Support Vars in List Metadata**
- Expose vars, settings, and env in ExtractMetadata()
- Enables column templates to access component variables
- Users can now use {{ .vars.region }}, {{ .vars.environment }}, etc.
- Follows same pattern as ExtractInstances()
**Testing:**
- Add TestDetectContentType_NoValue with comprehensive test cases
- Add TestExtractMetadata_IncludesVarsSettingsEnv to verify exposure
- Update existing metadata tests to include new fields
- All tests passing
**Impact:**
- list metadata now displays actual var values instead of <no value>
- Improves readability with dark gray styling for missing values
- Enables custom column configuration using vars/settings/env
Closes user request for dark gray <no value> styling and vars support.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/list/list_metadata_test.go (1)
285-309: Placeholder test for future filter implementation.This tests a stub function that currently returns nil. When filter functionality is implemented, these test cases should be expanded to validate actual filtering behavior.
pkg/list/list_instances_coverage_test.go (1)
61-67: Consider I/O initialization consistency across test variants.This test properly initializes I/O and UI contexts, but the error-path tests (
TestExecuteListInstancesCmd_InvalidConfigandTestExecuteListInstancesCmd_UploadPath) don't. If those tests fail early before reaching code that needs the I/O context, this is fine. However, this pattern is fragile—if error handling changes, tests could fail unexpectedly.Consider either:
- Adding a comment explaining why error-path tests don't need I/O initialization
- Extracting I/O setup to a test helper and using it consistently
- Initializing I/O in all ExecuteListInstancesCmd tests for consistency
📜 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)
cmd/list/stacks_test.go(2 hunks)pkg/list/list_instances_coverage_test.go(4 hunks)pkg/list/list_metadata_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: osterman
Repo: cloudposse/atmos PR: 1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
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: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
pkg/list/list_metadata_test.gopkg/list/list_instances_coverage_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/list/list_metadata_test.gocmd/list/stacks_test.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/list/list_metadata_test.gocmd/list/stacks_test.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
pkg/list/list_instances_coverage_test.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
pkg/list/list_instances_coverage_test.gocmd/list/stacks_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/list/list_instances_coverage_test.go
📚 Learning: 2024-10-20T00:41:57.135Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:93-98
Timestamp: 2024-10-20T00:41:57.135Z
Learning: When downloading schema files in `internal/exec/validate_stacks.go`, use a consistent temporary file name to overwrite the file each time and avoid creating multiple temporary files.
Applied to files:
cmd/list/stacks_test.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
cmd/list/stacks_test.go
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
cmd/list/stacks_test.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
cmd/list/stacks_test.go
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
cmd/list/stacks_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
cmd/list/stacks_test.go
🧬 Code graph analysis (3)
pkg/list/list_metadata_test.go (4)
errors/errors.go (1)
ErrInvalidConfig(43-43)pkg/schema/schema.go (3)
AtmosConfiguration(53-94)ListConfig(947-952)ListColumnConfig(954-957)pkg/list/sort/sort.go (5)
Sorter(35-39)NewSorter(48-54)Ascending(17-17)Descending(19-19)Order(13-13)pkg/list/filter/filter.go (1)
Filter(12-14)
pkg/list/list_instances_coverage_test.go (6)
pkg/io/context.go (1)
NewContext(22-46)pkg/ui/formatter.go (1)
InitFormatter(36-54)pkg/data/data.go (1)
InitWriter(29-35)pkg/list/sort/sort.go (6)
String(27-27)Sorter(35-39)NewSorter(48-54)Ascending(17-17)Descending(19-19)Order(13-13)pkg/list/list_instances.go (2)
ExecuteListInstancesCmd(349-456)InstancesCommandOptions(37-47)errors/errors.go (1)
ErrInvalidConfig(43-43)
cmd/list/stacks_test.go (2)
cmd/list/stacks.go (1)
StacksOptions(30-37)pkg/schema/schema.go (3)
AtmosConfiguration(53-94)ListConfig(947-952)ListColumnConfig(954-957)
⏰ 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 (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (20)
pkg/list/list_metadata_test.go (6)
1-14: LGTM!Clean test setup with appropriate imports and justified nolint directive.
16-124: Excellent test coverage for column flag parsing.Comprehensive test cases covering happy paths, edge cases (multiple equals signs, whitespace), and error conditions with proper error type checking.
126-211: Solid test coverage for column resolution precedence.Tests correctly validate the CLI flag → config → defaults hierarchy and handle edge cases like empty config columns.
213-283: Thorough testing of sorter construction.Covers default behavior, single/multiple columns, ordering directions, and error cases.
311-331: Good validation of default metadata columns.The hardcoded count and name list act as a safeguard against unintentional changes to the default columns configuration.
333-350: Basic struct validation test.While this only tests simple field assignment (which Go guarantees), it serves to document the MetadataOptions interface.
pkg/list/list_instances_coverage_test.go (7)
1-19: LGTM!The nolint directive is well-justified, and the new imports properly support the expanded test coverage for the rendering pipeline.
21-57: LGTM!These wrapper coverage tests are well-documented and appropriately scoped to verify execution without panics.
97-148: LGTM!Error path tests are appropriately structured and verify graceful failure handling for invalid configurations.
150-233: Excellent test coverage!This table-driven test thoroughly covers parsing edge cases including multiple equals signs in templates, empty values, and proper error wrapping. The use of
ErrorIsfor error type checking is spot-on.
235-321: LGTM!This test thoroughly validates the configuration precedence hierarchy and properly handles the empty-columns edge case. The fallback logic to defaults is well-tested.
323-408: LGTM!Comprehensive sorter test with good coverage of default behavior, explicit specifications, and error cases. The logic for returning
nilwhen columns don't match the default Component/Stack convention is correctly tested.
410-416: Placeholder test acknowledged.This test correctly validates the current placeholder behavior of
buildInstanceFilters. Remember to expand this test when the real filter implementation is added.cmd/list/stacks_test.go (7)
5-12: Clean imports for the new test coverage.The
stringsandschemapackages support the new test functions appropriately.
110-176: Solid coverage of all StacksOptions fields.The test cases cover meaningful combinations—fully populated, empty, and partial configurations. Structure is clear and aligns with the StacksOptions definition.
178-205: Test correctly validates filter construction.The expectation of 0 filters for the component case is well-documented—component filtering happens during extraction, not in the filter pipeline. This is clear from the comment.
207-281: Thorough testing of column resolution logic.Cases cover defaults (with/without component), CLI flags, and config-driven columns. The schema types align correctly with the codebase, and assertions validate both count and content.
283-330: Well-structured sorter tests.Covers empty (default), single field (both directions), multiple fields, and invalid specs. Error handling is verified alongside success cases.
332-347: Pragmatic approach to completion testing.The test acknowledges that full integration needs valid config, so it validates signature and directive behavior. The unused suggestions assignment is appropriately documented.
349-427: Thoughtful provenance validation testing.The test correctly differentiates between config loading errors and provenance validation failures. Smart use of
t.Skipfprevents false negatives when config issues occur before validation. The logic validates that provenance requires tree format while other formats should reject it.
- Fix doc comments in extract/components.go to start with symbol names - Remove unused ComponentData struct from extract/components.go - Fix stripBasePath to return relative, extensionless, forward-slash paths - Update provenance and resolver tests to use t.TempDir() and filepath.Join - Use strconv.Itoa for proper number formatting in long chain tests - Refactor list_instances.go tree format to avoid double stack processing - Add error for --upload with --format=tree combination - Fix stacks.go to call ProcessCommandLineArgs like other list subcommands - Add Width field to ListColumnConfig schema - Include Width in column config mapping across all list commands - Replace undefined contains() with u.SliceContainsString - Use canonical gopkg.in/yaml.v3 import instead of deprecated go.yaml.in 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add early validation for --provenance flag before config loading to prevent nil pointer dereference when cmd is nil. Skip test cases that require config loading since they cannot be tested without a real cmd. Add nolint directives to suppress complexity warnings in pkg/list/ and internal/exec/ files where functions have unavoidable complexity from nested logic patterns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
pkg/list/importresolver/provenance.go (1)
404-415: Fix relative import resolution to honor the importing file path
resolveImportPathcurrently ignoresparentFilePathand always resolves imports relative toStacksBaseAbsolutePath. For relative imports like./fooor../common/vars, this produces incorrect absolute paths (e.g., escaping the stacks directory) and diverges from the stack processor’s behavior.Use
parentFilePathfor./..imports and keep base-path resolution for “rooted” stack imports; also handle absolute import paths explicitly.-// resolveImportPath converts a relative import path to an absolute file path. -func resolveImportPath(importPath, _ string, atmosConfig *schema.AtmosConfiguration) string { - // Import paths are relative to the stacks base path. - basePath := atmosConfig.StacksBaseAbsolutePath - - // Add .yaml extension if not present. - if !strings.HasSuffix(importPath, yamlExt) && !strings.HasSuffix(importPath, ymlExt) { - importPath += yamlExt - } - - return filepath.Join(basePath, importPath) -} +// resolveImportPath converts an import path to an absolute file path. +// - Absolute paths are returned as-is (cleaned). +// - Relative paths starting with "." or ".." are resolved relative to the parent file directory. +// - All other paths are resolved relative to the stacks base path. +func resolveImportPath(importPath, parentFilePath string, atmosConfig *schema.AtmosConfiguration) string { + // Add .yaml extension if not present. + if !strings.HasSuffix(importPath, yamlExt) && !strings.HasSuffix(importPath, ymlExt) { + importPath += yamlExt + } + + // Absolute import: return cleaned path directly. + if filepath.IsAbs(importPath) { + return filepath.Clean(importPath) + } + + // Relative import (./ or ../): resolve from parent file directory when available. + if strings.HasPrefix(importPath, ".") && parentFilePath != "" { + parentDir := filepath.Dir(parentFilePath) + return filepath.Join(parentDir, importPath) + } + + // Fallback: resolve from stacks base path. + basePath := atmosConfig.StacksBaseAbsolutePath + return filepath.Join(basePath, importPath) +}cmd/list/components.go (1)
224-276:--columnsflag is effectively ignored (parseColumnsFlag is still a stub)
getComponentColumnstreats any non-emptycolumnsFlagas “use CLI columns”, butparseColumnsFlagignores the flag value and always returns the same defaultComponent/Stack/Typeset. That means:
--columnsandATMOS_LIST_COLUMNSdo not affect the output at all.- This contradicts the PR goal of configurable columns via CLI and atmos.yaml.
You should either make
--columnsfunctional now or fail fast when it’s used:
- Preferred: Implement real parsing that maps the requested column names to
column.Configentries (likely starting fromatmosConfig.Components.List.Columnsor a known default set), or- Interim: Detect non-empty
columnsFlagand return an error explaining that CLI column selection is not yet supported, rather than silently ignoring user input.For example, a conservative interim behavior:
func getComponentColumns(atmosConfig *schema.AtmosConfiguration, columnsFlag []string) []column.Config { // If --columns flag is provided, parse it and return. if len(columnsFlag) > 0 { - return parseColumnsFlag(columnsFlag) + // TODO: Implement full parsing of column specs. + // For now, fail fast instead of silently ignoring the flag. + panic("list components: --columns is wired but parsing is not yet implemented") }and then follow up with a real implementation that interprets
columnsFlaginto a filtered or reordered slice ofcolumn.Config.
🧹 Nitpick comments (1)
internal/exec/stack_processor_utils.go (1)
1056-1087: Provenance tracking logic works correctly.The import chain building logic is functional and the use of
u.SliceContainsStringis correct. The nested structure is acknowledged with the nolint directive.If you want to reduce duplication, the two branches (lines 1072-1077 and 1078-1084) could be consolidated since they perform identical operations:
- if len(result.mergeContext.ImportChain) > 0 && importedFile == result.mergeContext.ImportChain[0] { - // This is the imported file itself, add it only once. - if !u.SliceContainsString(mergeContext.ImportChain, importedFile) { - mergeContext.ImportChain = append(mergeContext.ImportChain, importedFile) - log.Trace("Added import to parent import chain", "file", importedFile) - } - } else { - // This is a nested import from the imported file. - if !u.SliceContainsString(mergeContext.ImportChain, importedFile) { - mergeContext.ImportChain = append(mergeContext.ImportChain, importedFile) - log.Trace("Added nested import to parent import chain", "file", importedFile) - } - } + if !u.SliceContainsString(mergeContext.ImportChain, importedFile) { + mergeContext.ImportChain = append(mergeContext.ImportChain, importedFile) + if len(result.mergeContext.ImportChain) > 0 && importedFile == result.mergeContext.ImportChain[0] { + log.Trace("Added import to parent import chain", "file", importedFile) + } else { + log.Trace("Added nested import to parent import chain", "file", importedFile) + } + }That said, the current structure makes the distinction between direct and nested imports clear, so this is purely a style preference.
📜 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 (13)
cmd/list/components.go(1 hunks)cmd/list/stacks.go(1 hunks)cmd/list/stacks_test.go(2 hunks)cmd/list/vendor.go(1 hunks)cmd/list/workflows.go(1 hunks)internal/exec/stack_processor_utils.go(31 hunks)pkg/list/column/column.go(1 hunks)pkg/list/extract/components.go(1 hunks)pkg/list/extract/workflows.go(1 hunks)pkg/list/importresolver/provenance.go(1 hunks)pkg/list/importresolver/provenance_test.go(1 hunks)pkg/list/importresolver/resolver_test.go(1 hunks)pkg/list/list_instances.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/list/importresolver/provenance_test.go
- cmd/list/stacks_test.go
🧰 Additional context used
🧠 Learnings (60)
📓 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: 1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands supports -1 for unlimited width, which bypasses terminal width restrictions and may require horizontal scrolling. The default is 10 columns if not specified.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
pkg/list/importresolver/resolver_test.go
📚 Learning: 2024-12-25T20:28:19.618Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/workflow_utils.go:167-169
Timestamp: 2024-12-25T20:28:19.618Z
Learning: The user plans to revert the change from `path.Join` to `filepath.Join` in this PR due to testing gaps and will open a new PR to safely handle the migration without breaking `main`.
Applied to files:
pkg/list/importresolver/resolver_test.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/list/importresolver/resolver_test.gopkg/list/extract/workflows.gointernal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.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/list/importresolver/resolver_test.gointernal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.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/list/importresolver/resolver_test.gocmd/list/workflows.gointernal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.gocmd/list/vendor.gocmd/list/stacks.go
📚 Learning: 2024-10-20T00:41:57.135Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:93-98
Timestamp: 2024-10-20T00:41:57.135Z
Learning: When downloading schema files in `internal/exec/validate_stacks.go`, use a consistent temporary file name to overwrite the file each time and avoid creating multiple temporary files.
Applied to files:
pkg/list/importresolver/resolver_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
pkg/list/importresolver/resolver_test.go
📚 Learning: 2025-02-05T11:10:51.031Z
Learnt from: mss
Repo: cloudposse/atmos PR: 1024
File: internal/exec/go_getter_utils.go:31-33
Timestamp: 2025-02-05T11:10:51.031Z
Learning: The path traversal check in `ValidateURI` function in `internal/exec/go_getter_utils.go` is intentionally kept despite potentially blocking valid Git URLs, as this validation is planned to be addressed in a separate ticket.
Applied to files:
pkg/list/importresolver/resolver_test.go
📚 Learning: 2024-11-22T12:38:33.132Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Applied to files:
pkg/list/importresolver/resolver_test.go
📚 Learning: 2025-09-24T20:45:40.401Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: tests/fixtures/scenarios/atmos-auth/stacks/deploy/nonprod.yaml:3-4
Timestamp: 2025-09-24T20:45:40.401Z
Learning: In Atmos stack files, the correct syntax for importing other stack files is `import:` (singular), not `imports:` (plural). All stack files in the Atmos codebase consistently use `import:` followed by a list of paths to import.
Applied to files:
pkg/list/importresolver/resolver_test.gopkg/list/importresolver/provenance.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/list/importresolver/resolver_test.gopkg/list/extract/workflows.gointernal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.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/list/list_instances.go
📚 Learning: 2025-06-07T19:28:21.289Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1266
File: cmd/describe_affected.go:0-0
Timestamp: 2025-06-07T19:28:21.289Z
Learning: In the Atmos codebase, using panic for unsupported flag types in flag processing functions like setDescribeAffectedFlagValueInCliArgs is the expected behavior rather than returning errors. This pattern is preferred for developer errors when unsupported types are added to the flagsKeyValue map.
Applied to files:
pkg/list/list_instances.gocmd/list/components.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/list/list_instances.gocmd/list/workflows.gocmd/list/components.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/list/list_instances.gocmd/list/components.go
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
pkg/list/list_instances.gocmd/list/stacks.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/list/list_instances.gocmd/list/workflows.gocmd/list/components.gocmd/list/vendor.gocmd/list/stacks.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/list/list_instances.gocmd/list/stacks.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Applied to files:
pkg/list/list_instances.gocmd/list/components.go
📚 Learning: 2025-02-03T06:00:11.419Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.
Applied to files:
pkg/list/list_instances.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/list/list_instances.gointernal/exec/stack_processor_utils.go
📚 Learning: 2025-02-20T13:57:36.326Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands (values, settings, metadata) supports -1 as a special value to display unlimited columns in table outputs. The default is 10 columns if not specified.
Applied to files:
pkg/list/list_instances.go
📚 Learning: 2025-02-20T13:57:36.326Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands supports -1 for unlimited width, which bypasses terminal width restrictions and may require horizontal scrolling. The default is 10 columns if not specified.
Applied to files:
pkg/list/list_instances.gocmd/list/vendor.go
📚 Learning: 2025-02-20T13:57:36.326Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands (values, settings, metadata) supports -1 as a special value to display unlimited columns in table outputs, with a default of 10 columns.
Applied to files:
pkg/list/list_instances.go
📚 Learning: 2025-02-20T13:57:36.326Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands supports -1 as a special value to display unlimited columns in table outputs.
Applied to files:
pkg/list/list_instances.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
pkg/list/list_instances.gointernal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.gocmd/list/vendor.gocmd/list/stacks.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/list/list_instances.gocmd/list/vendor.gocmd/list/stacks.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
pkg/list/list_instances.gocmd/list/components.gocmd/list/vendor.gopkg/list/column/column.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
cmd/list/workflows.gopkg/list/extract/workflows.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/list/workflows.gocmd/list/components.gointernal/exec/stack_processor_utils.gocmd/list/vendor.gocmd/list/stacks.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
cmd/list/workflows.gocmd/list/components.gocmd/list/stacks.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
cmd/list/workflows.gointernal/exec/stack_processor_utils.gocmd/list/stacks.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
cmd/list/workflows.gointernal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.gocmd/list/stacks.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
cmd/list/workflows.gointernal/exec/stack_processor_utils.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
cmd/list/workflows.gocmd/list/stacks.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
cmd/list/workflows.gointernal/exec/stack_processor_utils.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning the error instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions.
Applied to files:
pkg/list/extract/workflows.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
cmd/list/components.gointernal/exec/stack_processor_utils.gopkg/list/extract/components.gocmd/list/stacks.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/list/components.go
📚 Learning: 2024-10-31T07:09:31.983Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:181-182
Timestamp: 2024-10-31T07:09:31.983Z
Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.
Applied to files:
internal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.go
📚 Learning: 2025-10-16T05:26:31.892Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1639
File: pkg/merge/merge.go:392-430
Timestamp: 2025-10-16T05:26:31.892Z
Learning: In Atmos merge operations (pkg/merge/merge.go), context.Positions is a PositionMap (map[string]*schema.Position) keyed by JSONPath strings, not a slice indexed by input array position. Position lookups are path-based, making them independent of input array reorganization or filtering. This design allows filtering empty inputs without affecting provenance tracking correctness.
Applied to files:
internal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.go
📚 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:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1261
File: internal/exec/utils.go:639-640
Timestamp: 2025-05-22T15:42:10.906Z
Learning: In the Atmos codebase, when appending slices with `args := append(configAndStacksInfo.CliArgs, configAndStacksInfo.AdditionalArgsAndFlags...)`, it's intentional that the result is not stored back in the original slice. This pattern is used when the merged result serves a different purpose than the original slices, such as when creating a filtered version for component section assignments.
Applied to files:
internal/exec/stack_processor_utils.gocmd/list/stacks.go
📚 Learning: 2025-10-08T06:48:07.499Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1602
File: internal/exec/stack_processor_utils.go:968-1003
Timestamp: 2025-10-08T06:48:07.499Z
Learning: The `FindComponentDependenciesLegacy` function in `internal/exec/stack_processor_utils.go` is legacy code that is not actively used and is kept only for backward compatibility purposes.
Applied to files:
internal/exec/stack_processor_utils.gopkg/list/extract/components.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:
internal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.gocmd/list/stacks.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:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Applied to files:
internal/exec/stack_processor_utils.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:
internal/exec/stack_processor_utils.gopkg/list/importresolver/provenance.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:
internal/exec/stack_processor_utils.go
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-03-17T18:41:08.831Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/imports.go:68-75
Timestamp: 2025-03-17T18:41:08.831Z
Learning: In the Atmos configuration import system, errors during config file merging are logged at debug level and the process continues with other imports rather than failing completely, prioritizing resilience over strict correctness.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-04-03T10:57:04.602Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1091
File: pkg/config/load_config_args.go:61-82
Timestamp: 2025-04-03T10:57:04.602Z
Learning: In the Atmos configuration system, errors in `mergeDefaultImports` and `mergeImports` functions are intentionally only logged as debug messages without returning errors, as they are considered non-critical. Only errors in critical functions like `mergeConfigFile` are returned to halt execution.
Applied to files:
internal/exec/stack_processor_utils.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:
pkg/list/importresolver/provenance.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/list/importresolver/provenance.go
📚 Learning: 2025-10-02T19:17:51.630Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1504
File: pkg/profiler/profiler.go:20-31
Timestamp: 2025-10-02T19:17:51.630Z
Learning: In pkg/profiler/profiler.go, profiler-specific errors (ErrUnsupportedProfileType, ErrStartCPUProfile, ErrStartTraceProfile, ErrCreateProfileFile) must remain local and cannot be moved to errors/errors.go due to an import cycle: pkg/profiler → errors → pkg/schema → pkg/profiler. This is a valid exception to the centralized errors policy.
Applied to files:
pkg/list/importresolver/provenance.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:
cmd/list/vendor.go
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
cmd/list/stacks.go
📚 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:
cmd/list/stacks.go
🧬 Code graph analysis (11)
pkg/list/importresolver/resolver_test.go (3)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/list/importresolver/resolver.go (1)
ResolveImportTree(20-46)errors/errors.go (1)
ErrStackManifestFileNotFound(223-223)
pkg/list/list_instances.go (10)
pkg/list/column/column.go (3)
Config(13-17)NewSelector(52-79)BuildColumnFuncMap(278-304)pkg/schema/schema.go (3)
ConfigAndStacksInfo(624-718)AtmosConfiguration(53-94)Components(404-417)pkg/config/config.go (1)
InitCliConfig(28-67)pkg/list/format/formatter.go (2)
FormatTree(17-17)Format(8-8)internal/exec/stack_processor_utils.go (1)
ClearMergeContexts(74-80)internal/exec/utils.go (1)
ClearFindStacksMapCache(263-270)pkg/list/importresolver/provenance.go (1)
ResolveImportTreeFromProvenance(32-108)pkg/list/extract/metadata.go (1)
Metadata(43-53)pkg/list/renderer/renderer.go (1)
New(31-46)pkg/list/sort/sort.go (4)
Sorter(35-39)ParseSortSpec(195-237)NewSorter(48-54)Ascending(17-17)
cmd/list/workflows.go (8)
internal/exec/cli_utils.go (1)
ProcessCommandLineArgs(79-199)pkg/config/config.go (1)
InitCliConfig(28-67)pkg/list/extract/workflows.go (1)
Workflows(20-92)cmd/list/flag_wrappers.go (5)
NewListParser(304-315)WithFormatFlag(32-40)WithWorkflowsColumnsFlag(103-110)WithSortFlag(149-156)WithFileFlag(208-215)pkg/list/column/column.go (2)
NewSelector(52-79)BuildColumnFuncMap(278-304)pkg/list/renderer/renderer.go (1)
New(31-46)pkg/list/output/output.go (1)
New(14-16)pkg/list/sort/sort.go (4)
Sorter(35-39)NewSorter(48-54)Ascending(17-17)ParseSortSpec(195-237)
pkg/list/extract/workflows.go (4)
pkg/schema/schema.go (2)
Workflows(450-453)AtmosConfiguration(53-94)pkg/utils/file_utils.go (4)
IsYaml(48-59)IsPathAbsolute(100-102)IsDirectory(20-28)GetAllYamlFilesInDir(238-255)errors/errors.go (2)
ErrParseFile(12-12)ErrWorkflowDirectoryDoesNotExist(398-398)pkg/schema/workflow.go (1)
WorkflowManifest(24-28)
cmd/list/components.go (10)
pkg/flags/global_registry.go (1)
ParseGlobalFlags(33-74)pkg/config/config.go (1)
InitCliConfig(28-67)pkg/list/extract/components.go (1)
Components(17-44)cmd/list/flag_wrappers.go (9)
NewListParser(304-315)WithFormatFlag(32-40)WithComponentsColumnsFlag(81-86)WithSortFlag(149-156)WithStackFlag(126-133)WithTypeFlag(185-193)WithEnabledFlag(161-168)WithLockedFlag(173-180)WithAbstractFlag(241-248)errors/errors.go (2)
ErrInitializingCLIConfig(283-283)ErrExecuteDescribeStacks(404-404)pkg/list/column/column.go (3)
NewSelector(52-79)BuildColumnFuncMap(278-304)Config(13-17)pkg/list/renderer/renderer.go (1)
New(31-46)pkg/list/output/output.go (1)
New(14-16)pkg/list/filter/filter.go (4)
Filter(12-14)NewGlobFilter(40-58)NewColumnFilter(89-94)NewBoolFilter(121-126)pkg/list/sort/sort.go (4)
Sorter(35-39)NewSorter(48-54)Ascending(17-17)ParseSortSpec(195-237)
internal/exec/stack_processor_utils.go (8)
pkg/merge/merge_context.go (1)
MergeContext(16-36)pkg/perf/perf.go (1)
Track(121-138)pkg/logger/log.go (1)
Trace(14-16)pkg/utils/file_utils.go (1)
TrimBasePathFromPath(93-97)errors/errors.go (7)
ErrInvalidStackManifest(224-224)ErrInvalidOverridesSection(314-314)ErrInvalidTerraformSection(307-307)ErrInvalidTerraformOverridesSection(315-315)ErrInvalidHelmfileSection(308-308)ErrInvalidHelmfileOverridesSection(316-316)ErrInvalidImport(313-313)pkg/utils/json_utils.go (1)
ConvertFromJSON(170-187)pkg/config/const.go (3)
TerraformSectionName(79-79)OverridesSectionName(65-65)HelmfileSectionName(80-80)pkg/utils/slice_utils.go (1)
SliceContainsString(12-21)
pkg/list/importresolver/provenance.go (3)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/list/tree/types.go (1)
ImportNode(4-9)internal/exec/stack_processor_utils.go (1)
GetAllMergeContexts(84-96)
pkg/list/extract/components.go (2)
pkg/schema/schema.go (1)
Components(404-417)errors/errors.go (4)
ErrStackNotFound(412-412)ErrParseStacks(408-408)ErrParseComponents(409-409)ErrNoComponentsFound(410-410)
cmd/list/vendor.go (9)
pkg/flags/global_registry.go (1)
ParseGlobalFlags(33-74)pkg/config/config.go (1)
InitCliConfig(28-67)pkg/list/extract/vendor.go (1)
Vendor(24-44)cmd/list/flag_wrappers.go (5)
NewListParser(304-315)WithFormatFlag(32-40)WithVendorColumnsFlag(115-122)WithSortFlag(149-156)WithStackFlag(126-133)pkg/list/list_vendor.go (1)
GetVendorInfos(128-156)pkg/list/column/column.go (3)
NewSelector(52-79)BuildColumnFuncMap(278-304)Config(13-17)pkg/list/renderer/renderer.go (1)
New(31-46)pkg/list/filter/filter.go (2)
Filter(12-14)NewGlobFilter(40-58)pkg/list/sort/sort.go (4)
Sorter(35-39)NewSorter(48-54)Ascending(17-17)ParseSortSpec(195-237)
cmd/list/stacks.go (12)
internal/exec/cli_utils.go (1)
ProcessCommandLineArgs(79-199)pkg/config/config.go (1)
InitCliConfig(28-67)pkg/schema/schema.go (2)
Stacks(441-448)AtmosConfiguration(53-94)pkg/list/extract/stacks.go (2)
Stacks(11-27)StacksForComponent(30-73)cmd/list/flag_wrappers.go (6)
NewListParser(304-315)WithFormatFlag(32-40)WithStacksColumnsFlag(91-98)WithSortFlag(149-156)WithComponentFlag(197-204)WithProvenanceFlag(285-292)errors/errors.go (3)
ErrInvalidFlag(147-147)ErrInitializingCLIConfig(283-283)ErrExecuteDescribeStacks(404-404)pkg/list/importresolver/provenance.go (1)
ResolveImportTreeFromProvenance(32-108)pkg/list/tree/types.go (1)
ImportNode(4-9)pkg/list/format/tree_stacks.go (1)
RenderStacksTree(14-25)pkg/list/column/column.go (3)
NewSelector(52-79)BuildColumnFuncMap(278-304)Config(13-17)pkg/list/renderer/renderer.go (1)
New(31-46)pkg/list/sort/sort.go (4)
Sorter(35-39)NewSorter(48-54)Ascending(17-17)ParseSortSpec(195-237)
pkg/list/column/column.go (4)
pkg/list/extract/metadata.go (1)
Metadata(43-53)internal/exec/template_funcs.go (1)
FuncMap(21-34)errors/errors.go (2)
ErrInvalidConfig(43-43)ErrTemplateEvaluation(42-42)pkg/list/renderer/renderer.go (1)
New(31-46)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (8)
internal/exec/stack_processor_utils.go (2)
82-96: LGTM! Clean defensive copy pattern.The new function properly locks for thread safety and returns a defensive copy to prevent external modifications. The implementation aligns well with the existing merge context API.
19-19: Good provenance tracking instrumentation.The trace logging additions provide helpful visibility into merge context propagation and import chain building. The logger alias is used correctly throughout.
Also applies to: 616-627, 877-878
pkg/list/importresolver/resolver_test.go (1)
16-912: Tests look solid and cross‑platform friendlyThe resolver tests cover the key behaviors (empty stacks, nested imports, circular detection, caching, path resolution, and error paths) and use
filepath.Join/t.TempDir()correctly, so they should behave well on all platforms.cmd/list/workflows.go (1)
114-217: Workflows command implementation looks solid.The renderer pipeline integration is clean: ProcessCommandLineArgs properly flows CLI config through to InitCliConfig, columns are handled as a string slice matching the flag definition, and the default columns use correct template variables (
.file,.name,.description,.steps). All past review concerns have been addressed.The pattern here—dynamic completion, flag wrappers, column/filter/sorter builders, and renderer delegation—matches the broader list command refactor nicely.
pkg/list/column/column.go (1)
52-79: Column selector architecture is well-designed.Pre-parsing templates at selector creation (with validation) and deferring evaluation until Extract keeps the happy path fast while catching config errors early. The comprehensive field mapping in buildTemplateContext and safe template function set make this a solid foundation for the rendering pipeline.
pkg/list/list_instances.go (1)
352-464: Instance listing execution flow is clean.The tree format branch now processes stacks exactly once (line 390), enables provenance correctly, and returns early (line 406) before the upload path—fixing the double-processing concern from past reviews. Format and upload flag error handling is consistent, and the renderer wiring (filters/selector/sorters) sets up the pipeline correctly even though filters are currently a no-op.
cmd/list/vendor.go (1)
185-212: Column width preservation looks good.
getVendorColumnsnow correctly copies theWidthfield fromatmosConfig.Vendor.List.Columnsat line 199, addressing the past review concern about dropped width settings.cmd/list/stacks.go (1)
122-252: Stacks command correctly integrates CLI config and provenance.The two-stage provenance validation (lines 125-127 and 145-148) catches misuse early when
--formatis explicit while still respecting config defaults.ProcessCommandLineArgsat line 130 ensures CLI flags like--base-pathare honored, matching the pattern in other list commands. The tree format correctly filters by component (lines 198-220), preventing the "show all stacks" issue from past reviews.Clean implementation that addresses all previous concerns.
4 tasks
The stackComponents map was being built but never read or used in the ResolveImportTreeFromProvenance function. This removes the dead code. Also updated nolint directive since function is now shorter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add perf.Track instrumentation to all list extraction and resolver functions - pkg/list/extract/workflows.go: Workflows, extractFromManifest - pkg/list/extract/components.go: Components, ComponentsForStack, extractComponentType, enrichComponentWithMetadata - cmd/list/components.go: All helper functions (columnsCompletionForComponents, listComponentsWithOptions, buildComponentFilters, getComponentColumns, buildComponentSorters, parseColumnsFlag) - pkg/list/importresolver/provenance.go: All functions (ResolveImportTreeFromProvenance, findStacksForFilePath, extractComponentFolders, extractComponentsFromStackData, buildImportTreeFromChain, resolveImportFileImports, buildNodesFromImportPaths, resolveImportPath, readImportsFromYAMLFile, extractImportStringsHelper) - Fix vendor stack filter to target correct column - Changed from "stack" to "component" since vendor rows contain component, type, manifest, folder fields - Fix Type filter in components command - Changed filter target from "type" to "component_type" - Made Type filter authoritative: when --type is provided, no default abstract filter is added - Only applies default abstract filter when Type is not set and Abstract is false - Fix relative import path resolution in provenance resolver - resolveImportPath now detects relative imports (starting with . or ..) - Relative imports resolve against parent file's directory - Non-relative imports continue to resolve against stacks base path - Update component filter tests to match new Type filter behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
cmd/list/components.go (1)
273-287: Consider implementing column parsing or document the deferral.The
parseColumnsFlagstub always returns defaults despite the--columnsflag being wired. The TODO comment andnolint:unparamindicate this is intentionally deferred, but users expecting custom columns via CLI will still get the default set. Either implement the parsing logic to map flag values to column specs, or add a temporary user-facing warning when the flag is passed.cmd/list/vendor.go (1)
170-184: Consider renamingopts.Stackto clarify vendor filtering.While the implementation correctly filters the
"component"field (line 177), the parameter nameopts.Stackis misleading since vendor rows don't contain stack data. This could confuse future maintainers. Consider either renamingVendorOptions.StacktoComponentorFilter, or adding a comment explaining the field repurposing. The comment already clarifies the intent ("Component filter"), but the field name doesn't match.pkg/list/extract/workflows.go (1)
55-59: Consider extracting path construction logic.The absolute path check and join pattern appears in both locations. A small helper would reduce duplication and make the intent clearer.
Something like:
func resolveWorkflowPath(atmosConfig *schema.AtmosConfiguration, relativePath string) string { if utils.IsPathAbsolute(atmosConfig.Workflows.BasePath) { return filepath.Join(atmosConfig.Workflows.BasePath, relativePath) } return filepath.Join(atmosConfig.BasePath, atmosConfig.Workflows.BasePath, relativePath) }Then use it at lines 55-59 and 74-78. No pressure though—the current approach works fine.
Also applies to: 74-78
pkg/list/extract/components.go (2)
78-95: Preserve metadata invariants when componentData has an unexpected shapeRight now, if
componentDataisn’t amap[string]any,enrichComponentWithMetadatareturns immediately, leaving the component withoutenabled,locked, orcomponent_typefields and without adatafield. That can make list filters/columns behave inconsistently for malformed components.You can at least apply default metadata before bailing out so callers always see a consistent shape:
func enrichComponentWithMetadata(comp map[string]any, componentData any) { defer perf.Track(nil, "list.extract.enrichComponentWithMetadata")() compMap, ok := componentData.(map[string]any) if !ok { - return + // Ensure callers still see default metadata fields even for malformed components. + setDefaultMetadataFields(comp) + return }This keeps the “always have enabled/locked/component_type” invariant without changing error semantics.
21-47: Confirm intentional difference in “no components” behavior between aggregate and per-stack paths
Componentsreturns an empty slice andnilerror when no components are found across all stacks, whileComponentsForStackreturnsErrNoComponentsFoundwhen a specific stack has zero components.If that distinction is intentional (aggregate listing treats “nothing to list” as success, targeted listing treats “nothing for this stack” as an error), it might be worth making that explicit in the doc comments so callers don’t misinterpret a
nil,nilfromComponentsas an error/edge case.If you intended uniform behavior instead, you could either:
- have
Componentsalso returnErrNoComponentsFoundwhenlen(components) == 0, or- relax
ComponentsForStackto match the aggregate behavior and just return an empty slice.Given the surrounding pipeline, I suspect the current behavior is fine; mostly asking you to double-check it’s by design.
Also applies to: 147-153
📜 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 (6)
cmd/list/components.go(1 hunks)cmd/list/components_test.go(2 hunks)cmd/list/vendor.go(1 hunks)pkg/list/extract/components.go(1 hunks)pkg/list/extract/workflows.go(1 hunks)pkg/list/importresolver/provenance.go(1 hunks)
🧰 Additional context used
🧠 Learnings (36)
📓 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: 1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands supports -1 for unlimited width, which bypasses terminal width restrictions and may require horizontal scrolling. The default is 10 columns if not specified.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
pkg/list/extract/workflows.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning the error instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions.
Applied to files:
pkg/list/extract/workflows.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/list/extract/workflows.gopkg/list/importresolver/provenance.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/list/extract/workflows.gopkg/list/importresolver/provenance.gocmd/list/vendor.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/list/extract/workflows.gocmd/list/components.gopkg/list/importresolver/provenance.gopkg/list/extract/components.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/list/extract/workflows.gocmd/list/components.gopkg/list/importresolver/provenance.gopkg/list/extract/components.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/list/extract/workflows.gopkg/list/importresolver/provenance.gopkg/list/extract/components.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
cmd/list/components.gopkg/list/extract/components.gocmd/list/components_test.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
cmd/list/components.gocmd/list/vendor.gocmd/list/components_test.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/list/components.gocmd/list/vendor.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
cmd/list/components.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
cmd/list/components.gocmd/list/vendor.go
📚 Learning: 2025-01-07T20:38:09.618Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 896
File: cmd/editor_config.go:37-40
Timestamp: 2025-01-07T20:38:09.618Z
Learning: Error handling suggestion for `cmd.Help()` in `cmd/editor_config.go` was deferred as the code is planned for future modifications.
Applied to files:
cmd/list/components.go
📚 Learning: 2025-06-07T19:28:21.289Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1266
File: cmd/describe_affected.go:0-0
Timestamp: 2025-06-07T19:28:21.289Z
Learning: In the Atmos codebase, using panic for unsupported flag types in flag processing functions like setDescribeAffectedFlagValueInCliArgs is the expected behavior rather than returning errors. This pattern is preferred for developer errors when unsupported types are added to the flagsKeyValue map.
Applied to files:
cmd/list/components.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/list/components.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:
cmd/list/components.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Applied to files:
cmd/list/components.gocmd/list/components_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/list/importresolver/provenance.gocmd/list/vendor.go
📚 Learning: 2025-10-02T19:17:51.630Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1504
File: pkg/profiler/profiler.go:20-31
Timestamp: 2025-10-02T19:17:51.630Z
Learning: In pkg/profiler/profiler.go, profiler-specific errors (ErrUnsupportedProfileType, ErrStartCPUProfile, ErrStartTraceProfile, ErrCreateProfileFile) must remain local and cannot be moved to errors/errors.go due to an import cycle: pkg/profiler → errors → pkg/schema → pkg/profiler. This is a valid exception to the centralized errors policy.
Applied to files:
pkg/list/importresolver/provenance.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/list/importresolver/provenance.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:
pkg/list/importresolver/provenance.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
pkg/list/importresolver/provenance.gocmd/list/vendor.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/list/importresolver/provenance.go
📚 Learning: 2025-09-24T20:45:40.401Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: tests/fixtures/scenarios/atmos-auth/stacks/deploy/nonprod.yaml:3-4
Timestamp: 2025-09-24T20:45:40.401Z
Learning: In Atmos stack files, the correct syntax for importing other stack files is `import:` (singular), not `imports:` (plural). All stack files in the Atmos codebase consistently use `import:` followed by a list of paths to import.
Applied to files:
pkg/list/importresolver/provenance.go
📚 Learning: 2025-10-16T05:26:31.892Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1639
File: pkg/merge/merge.go:392-430
Timestamp: 2025-10-16T05:26:31.892Z
Learning: In Atmos merge operations (pkg/merge/merge.go), context.Positions is a PositionMap (map[string]*schema.Position) keyed by JSONPath strings, not a slice indexed by input array position. Position lookups are path-based, making them independent of input array reorganization or filtering. This design allows filtering empty inputs without affecting provenance tracking correctness.
Applied to files:
pkg/list/importresolver/provenance.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:
cmd/list/vendor.go
📚 Learning: 2025-02-20T13:57:36.326Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands supports -1 for unlimited width, which bypasses terminal width restrictions and may require horizontal scrolling. The default is 10 columns if not specified.
Applied to files:
cmd/list/vendor.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:
cmd/list/vendor.go
📚 Learning: 2024-10-22T23:00:20.627Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Applied to files:
cmd/list/vendor.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
cmd/list/vendor.go
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
cmd/list/vendor.go
📚 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:
cmd/list/vendor.go
📚 Learning: 2025-10-08T06:48:07.499Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1602
File: internal/exec/stack_processor_utils.go:968-1003
Timestamp: 2025-10-08T06:48:07.499Z
Learning: The `FindComponentDependenciesLegacy` function in `internal/exec/stack_processor_utils.go` is legacy code that is not actively used and is kept only for backward compatibility purposes.
Applied to files:
pkg/list/extract/components.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
cmd/list/components_test.go
📚 Learning: 2025-02-09T14:38:53.443Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 992
File: cmd/cmd_utils.go:0-0
Timestamp: 2025-02-09T14:38:53.443Z
Learning: Error handling for RegisterFlagCompletionFunc in AddStackCompletion is not required as the errors are non-critical for tab completion functionality.
Applied to files:
cmd/list/components_test.go
🧬 Code graph analysis (3)
pkg/list/extract/workflows.go (4)
pkg/perf/perf.go (1)
Track(121-138)pkg/utils/file_utils.go (3)
IsYaml(48-59)IsPathAbsolute(100-102)GetAllYamlFilesInDir(238-255)errors/errors.go (2)
ErrParseFile(19-19)ErrWorkflowDirectoryDoesNotExist(405-405)pkg/schema/workflow.go (1)
WorkflowManifest(24-28)
pkg/list/importresolver/provenance.go (4)
pkg/list/tree/types.go (1)
ImportNode(4-9)pkg/perf/perf.go (1)
Track(121-138)internal/exec/stack_processor_utils.go (1)
GetAllMergeContexts(84-96)pkg/logger/log.go (1)
Trace(14-16)
pkg/list/extract/components.go (3)
pkg/schema/schema.go (1)
Components(404-417)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (4)
ErrStackNotFound(419-419)ErrParseStacks(415-415)ErrParseComponents(416-416)ErrNoComponentsFound(417-417)
⏰ 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 (4)
pkg/list/importresolver/provenance.go (3)
33-101: Import tree resolution logic is solid.The provenance-based resolution correctly handles circular imports with backtracking, implements per-call caching to optimize repeated file reads, and properly attaches component folder metadata to first-level import nodes. The fixed relative path handling (using parent directory for paths starting with ".") aligns with standard Atmos import semantics.
Also applies to: 273-315, 371-407
107-187: File path matching handles multiple representations correctly.The multi-strategy approach (absolute, relative, extension-agnostic) at lines 172-176 provides good flexibility for matching
atmos_stack_filevalues that may be stored in different formats. The normalization and type-safety checks throughout are appropriate.
240-271: Function is actively tested—no removal needed.The search shows
extractComponentsFromStackDatais called inpkg/list/importresolver/provenance_test.go:353. It has dedicated test coverage and proper instrumentation. No changes required.Likely an incorrect or invalid review comment.
pkg/list/extract/workflows.go (1)
21-22: Performance tracking properly implemented.Both entry points now have
perf.Trackinstrumentation as requested in the previous review. The use ofatmosConfiginWorkflowsandnilinextractFromManifestis appropriate given their respective signatures.Also applies to: 98-99
Member
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Add perf.Track instrumentation to all helper functions in vendor command and list extract packages to satisfy repository performance tracking policy. Changes: - cmd/list/vendor.go: Add perf tracking to columnsCompletionForVendor, listVendorWithOptions, buildVendorFilters, getVendorColumns, and buildVendorSorters functions - pkg/list/extract/components.go: Add perf tracking to buildBaseComponent, extractMetadataFields, setDefaultMetadataFields, getBoolWithDefault, and getStringWithDefault helper functions - pkg/list/extract/workflows.go: Fix cleanPath usage to ensure normalized path is used consistently for all file operations and error messages - pkg/list/importresolver/provenance.go: Add perf tracking to stripBasePath function All functions now have defer perf.Track(nil, "package.functionName")() as the first statement, matching the repository's mandatory performance tracking pattern. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The stripBasePath function was removing file extensions before converting path separators to forward slashes, which caused inconsistent behavior across different operating systems. Changes: - Reorder operations to convert to forward slashes first (via filepath.ToSlash) - Then remove .yaml/.yml extensions using string literals instead of constants - This ensures consistent relative, extensionless paths regardless of OS The fix ensures that stripBasePath returns paths like "catalog/base" rather than OS-specific separator formats, making the import tree rendering consistent across Windows, macOS, and Linux. All tests now pass, including TestStripBasePath which validates: - Simple paths - Paths with trailing slashes - Both .yaml and .yml extensions - Relative paths - Nested directory structures 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…omponent - Replace outdated `:::note Purpose` pattern in CLAUDE.md documentation template with modern `<Intro>` component - Add Intro import to template example - Update terminology from "purpose note" to "intro" for consistency - Improve list-stacks intro to describe specific user actions (filtering, format options, understanding inheritance) instead of generic "list stacks" - Follows pattern from auth commands: concrete actions users can accomplish This fixes the root cause of documentation hallucinations where new docs were being written with deprecated :::note syntax. Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace deprecated `:::note Purpose` admonitions with modern `<Intro>` component across all list command documentation files. Update intros to describe specific user actions and benefits following pattern from auth commands. Changes: - list-components.mdx: Add Intro describing filtering and output formats - list-instances.mdx: Add Intro explaining component-stack combinations and Atmos Pro upload - list-metadata.mdx: Add Intro about filtering and sorting metadata fields - list-vendor.mdx: Add Intro describing vendor sources and dependencies - themes.mdx: Add Intro about color schemes and syntax highlighting - list-workflows.mdx: Add Intro, Screengrab, and description field - list-settings.mdx: Add Intro about cross-stack comparison tables - list-values.mdx: Add Intro about configuration value comparison - list-vars.mdx: Add Intro about Terraform variable comparison All intros now: - Use action-oriented language (what users can accomplish) - Describe specific capabilities (filtering, formats, comparison) - Follow modern documentation pattern with <Intro> component - Include Screengrab components where missing Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/list/extract/workflows.go (1)
72-78: Consider extracting path construction helperThe logic here duplicates lines 55-59. Both blocks check
IsPathAbsoluteand construct paths accordingly. A small helper function could reduce repetition:func buildWorkflowPath(atmosConfig *schema.AtmosConfiguration, relPath string) string { if utils.IsPathAbsolute(atmosConfig.Workflows.BasePath) { return filepath.Join(atmosConfig.Workflows.BasePath, relPath) } return filepath.Join(atmosConfig.BasePath, atmosConfig.Workflows.BasePath, relPath) }Then both call sites become simpler. Optional cleanup—the current approach works fine.
pkg/list/importresolver/provenance.go (1)
346-371: Consider tracing YAML read/parse failures inresolveImportFileImportsRight now, any error from
readImportsFromYAMLFileis silently treated as “no imports”, which can make it harder to debug missing branches in the provenance tree when a path is wrong or a file is malformed. A low-noiseTracelog would help operators understand why parts of the tree disappear without impacting normal runs.For example:
func resolveImportFileImports( importFilePath string, atmosConfig *schema.AtmosConfiguration, visited map[string]bool, cache map[string][]string, ) []*tree.ImportNode { @@ - // Read imports from the file. - imports, err := readImportsFromYAMLFile(importFilePath) - if err != nil { - // File can't be read or has no imports. - return nil - } + // Read imports from the file. + imports, err := readImportsFromYAMLFile(importFilePath) + if err != nil { + // Trace-level log so provenance debugging can see why a branch is missing. + log.Trace("Failed to read imports from YAML file", "file", importFilePath, "error", err) + return nil + }
📜 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 (5)
CLAUDE.md(4 hunks)cmd/list/vendor.go(1 hunks)pkg/list/extract/components.go(1 hunks)pkg/list/extract/workflows.go(1 hunks)pkg/list/importresolver/provenance.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/list/extract/components.go
🧰 Additional context used
🧠 Learnings (41)
📓 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: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
pkg/list/extract/workflows.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/list/extract/workflows.gopkg/list/importresolver/provenance.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/list/extract/workflows.gopkg/list/importresolver/provenance.gocmd/list/vendor.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/list/extract/workflows.gopkg/list/importresolver/provenance.gocmd/list/vendor.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/list/extract/workflows.gopkg/list/importresolver/provenance.gocmd/list/vendor.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/list/extract/workflows.gopkg/list/importresolver/provenance.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/list/extract/workflows.gopkg/list/importresolver/provenance.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
pkg/list/extract/workflows.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:
pkg/list/extract/workflows.go
📚 Learning: 2024-10-27T04:41:49.199Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:215-223
Timestamp: 2024-10-27T04:41:49.199Z
Learning: In `internal/exec/terraform_clean.go`, the function `determineCleanPath` is necessary and should not be removed.
Applied to files:
pkg/list/extract/workflows.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
pkg/list/extract/workflows.go
📚 Learning: 2024-12-25T20:28:19.618Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/workflow_utils.go:167-169
Timestamp: 2024-12-25T20:28:19.618Z
Learning: The user plans to revert the change from `path.Join` to `filepath.Join` in this PR due to testing gaps and will open a new PR to safely handle the migration without breaking `main`.
Applied to files:
pkg/list/extract/workflows.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
pkg/list/extract/workflows.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/list/extract/workflows.gopkg/list/importresolver/provenance.gocmd/list/vendor.go
📚 Learning: 2025-11-07T14:52:55.217Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1761
File: docs/prd/claude-agent-architecture.md:331-439
Timestamp: 2025-11-07T14:52:55.217Z
Learning: In the cloudposse/atmos repository, Claude agents are used as interactive tools, not in automated/headless CI/CD contexts. Agent documentation and patterns should assume synchronous human interaction.
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.md
📚 Learning: 2024-11-10T19:28:17.365Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:0-0
Timestamp: 2024-11-10T19:28:17.365Z
Learning: When TTY is not supported, log the downgrade message at the Warn level using `u.LogWarning(cliConfig, ...)` instead of `fmt.Println`.
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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:
CLAUDE.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:
CLAUDE.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:
CLAUDE.md
📚 Learning: 2025-10-14T01:54:48.410Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-completion--help.html:2-7
Timestamp: 2025-10-14T01:54:48.410Z
Learning: Screengrab HTML files in website/src/components/Screengrabs/ are generated from actual Atmos CLI output converted to HTML. The ANSI-art headers and formatting in these files are intentional and reflect the real CLI user experience, so they should not be suggested for removal or modification.
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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:
CLAUDE.md
📚 Learning: 2025-10-02T19:17:51.630Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1504
File: pkg/profiler/profiler.go:20-31
Timestamp: 2025-10-02T19:17:51.630Z
Learning: In pkg/profiler/profiler.go, profiler-specific errors (ErrUnsupportedProfileType, ErrStartCPUProfile, ErrStartTraceProfile, ErrCreateProfileFile) must remain local and cannot be moved to errors/errors.go due to an import cycle: pkg/profiler → errors → pkg/schema → pkg/profiler. This is a valid exception to the centralized errors policy.
Applied to files:
pkg/list/importresolver/provenance.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:
pkg/list/importresolver/provenance.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/list/importresolver/provenance.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
pkg/list/importresolver/provenance.gocmd/list/vendor.go
📚 Learning: 2025-09-24T20:45:40.401Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: tests/fixtures/scenarios/atmos-auth/stacks/deploy/nonprod.yaml:3-4
Timestamp: 2025-09-24T20:45:40.401Z
Learning: In Atmos stack files, the correct syntax for importing other stack files is `import:` (singular), not `imports:` (plural). All stack files in the Atmos codebase consistently use `import:` followed by a list of paths to import.
Applied to files:
pkg/list/importresolver/provenance.go
📚 Learning: 2024-11-22T12:38:33.132Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Applied to files:
pkg/list/importresolver/provenance.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/list/importresolver/provenance.go
📚 Learning: 2025-10-16T05:26:31.892Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1639
File: pkg/merge/merge.go:392-430
Timestamp: 2025-10-16T05:26:31.892Z
Learning: In Atmos merge operations (pkg/merge/merge.go), context.Positions is a PositionMap (map[string]*schema.Position) keyed by JSONPath strings, not a slice indexed by input array position. Position lookups are path-based, making them independent of input array reorganization or filtering. This design allows filtering empty inputs without affecting provenance tracking correctness.
Applied to files:
pkg/list/importresolver/provenance.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/list/vendor.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:
cmd/list/vendor.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
cmd/list/vendor.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
cmd/list/vendor.go
📚 Learning: 2025-02-20T13:57:36.326Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:36.326Z
Learning: The max-columns flag in Atmos list commands supports -1 for unlimited width, which bypasses terminal width restrictions and may require horizontal scrolling. The default is 10 columns if not specified.
Applied to files:
cmd/list/vendor.go
📚 Learning: 2024-10-22T23:00:20.627Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Applied to files:
cmd/list/vendor.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
cmd/list/vendor.go
🧬 Code graph analysis (1)
cmd/list/vendor.go (11)
pkg/flags/standard_parser.go (1)
StandardParser(18-22)pkg/flags/global_registry.go (1)
ParseGlobalFlags(33-74)pkg/schema/schema.go (3)
ConfigAndStacksInfo(624-718)Vendor(936-941)AtmosConfiguration(53-94)pkg/config/config.go (1)
InitCliConfig(28-67)pkg/list/extract/vendor.go (1)
Vendor(24-44)cmd/list/flag_wrappers.go (2)
NewListParser(304-315)WithFormatFlag(32-40)pkg/list/list_vendor.go (1)
GetVendorInfos(128-156)pkg/list/column/column.go (2)
NewSelector(52-79)Config(13-17)pkg/list/renderer/renderer.go (1)
New(31-46)pkg/list/output/output.go (1)
New(14-16)pkg/list/sort/sort.go (4)
Sorter(35-39)NewSorter(48-54)Ascending(17-17)ParseSortSpec(195-237)
🪛 LanguageTool
CLAUDE.md
[typographical] ~40-~40: Consider using a typographic close quote here.
Context: ...i-expert(terminal UI). Use Task tool:Task(subagent_type: "flag-handler", prompt: "...")`. Prevents incorrect patterns and ensu...
(EN_QUOTES)
⏰ 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 (20)
CLAUDE.md (3)
39-40: Specialized Agents guidance is clear and actionable.The new mandatory pattern for consulting specialized agents via the Task tool provides concrete direction. The specific mention of
flag-handleraligns well with the PR's list command refactoring and prevents duplicated effort.
767-790: Intro component integration strengthens documentation consistency.The addition of Intro component usage in the documentation template aligns the guidance with the PR's UI/messaging improvements across list commands. The example clearly shows the intended pattern, and the section ordering (Intro → Screengrab → Usage → Examples) reads naturally.
206-206: Benefits consolidation maintains substance while improving clarity.Replacing the multi-block rationale with a single concise line ("Auto color/width degradation, TTY detection, secret masking, ...") preserves the key benefits without sacrificing readability. This is especially valuable in a document this size.
pkg/list/extract/workflows.go (2)
3-26: Clean structure and importsThe imports are correct, perf tracking is in place, and the linter suppressions are justified for the file handling complexity. Good start.
97-120: Solid extraction logicThe function is well-structured with perf tracking, nil-checking, and clear mapping from manifest to output format. Works as intended.
pkg/list/importresolver/provenance.go (9)
33-101: Provenance tree assembly and perf instrumentation look solidThe aggregation over merge contexts into
map[stackName]map[componentName][]*tree.ImportNodeis coherent, and the use ofextractComponentFoldersto attachComponentFolderon the first node per component keeps rendering concerns nicely separated.perf.Trackis wired in as per policy and there are no obvious correctness issues here.
107-187: Stack/file matching logic infindStacksForFilePathis thoroughThe normalization plus multiple match strategies (absolute, relative, and extensionless) give this a good chance of matching
atmos_stack_filevalues across common variants without overcomplicating things. The function is defensive around type assertions and keeps logging atTrace, which is appropriate.
191-238: Component folder derivation aligns with components/{type}/{folder} conventionUsing
metadata.componentwhen present and falling back to the component name, then composingcomponents/{type}/{folder}, matches Atmos’ component folder semantics and gives renderers the path they need without exposing the full stack structure.
241-271:extractComponentsFromStackDatais a simple, reusable helperThis helper cleanly extracts component names across all types and is defensive on shapes. It’s a reasonable internal/test utility to keep component enumeration logic in one place.
273-315: ImportChain → tree conversion is cycle-safe and cached
buildImportTreeFromChaincorrectly skips the root stack entry, normalizes display paths viastripBasePath, and uses avisitedset plusimportCacheto avoid infinite recursion and redundant file reads. The backtracking ofvisitedper branch looks correct.
319-344:stripBasePathnormalization and extension stripping are well-behavedThe combination of
filepath.Relwith a TrimPrefix fallback, plusfilepath.ToSlashand.yaml/.ymlstripping, gives consistent, cross-platform, extensionless display paths. The addedperf.Trackkeeps it aligned with repo profiling rules.
373-409: Child-nodePathvalues intentionally preserve original import stringsTop-level nodes are normalized via
stripBasePath, while child nodes use the rawimportPathfrom YAML forPath. That gives users what they actually wrote in nested imports, at the expense of inconsistent normalization between levels. If that’s the intended UX for--provenanceoutput, this implementation supports it cleanly.
411-439: Import path resolution correctly handles relative vs base-path importsThe split between
./..-prefixed paths resolved againstparentFilePathand other imports resolved againstStacksBaseAbsolutePathmatches expected stack import semantics and keeps cycle detection keyed on absolute paths. Path cleaning and extension handling look correct, and directory traversal for legitimate..imports is preserved as intended.
Based on learnings, directory traversal in stack imports is acceptable here.
441-492: YAML import extraction is flexible and defensive
readImportsFromYAMLFileplusextractImportStringsHelperhandle bothimportandimports, and accept either a single string or a slice, returning an empty slice instead ofnilwhen no imports are found. That makes the callers simpler and keeps provenance resolution tolerant of optional import fields. The perf tracking is in place on both helpers.cmd/list/vendor.go (6)
68-91: LGTM!Dynamic completion properly loads config and extracts vendor column names. Performance tracking is in place and error handling is graceful.
93-117: Clean flag setup with modular wrappers.Flag configuration uses composable wrappers and properly registers completions and Viper bindings. Follows established patterns.
119-173: Solid renderer pipeline implementation.The flow is clean: config → vendor data → filters/columns/sorters → renderer → output. Format fallback from config and graceful handling of empty vendors are good touches. Error handling is comprehensive.
175-191: Filter implementation correctly targets component field.The filter logic and inline comment clearly document that
--stackis applied to the"component"field (vendor rows don't contain stack context). This design was reviewed in past comments and accepted.
193-222: Column configuration properly preserves width settings.Flag precedence is correct (flag → config → defaults), and the Width field is now carried through from
atmos.yamlconfiguration as intended. Default columns align with vendor data structure.
224-236: Sorter configuration with sensible defaults.Default sort by Component ascending is appropriate, and the implementation delegates to the shared
ParseSortSpecvalidation. Clean and correct.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
what
atmos.yamland--columnsCLI flag with Go template support--format=treeand--provenanceflags for import hierarchy displaywhy
--columns "Name={{ .component }}")references
Key Features
Configurable Columns
CLI Flags
--columns- Custom column definitions with Go templates--format- Output format (table, json, yaml, csv, tsv, tree)--sort- Sort specification (e.g.,Stack:asc,Component:desc)--provenance- Show import hierarchy in tree formatTest Coverage
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UX / Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.