Repository navigation
feat(list): add --skip flag; fix --stack/--filter/--query on list instances - #2413
Merged
Merged
Conversation
…tances Adds --skip <yaml-function> across every `atmos list` subcommand that processes stacks (instances, components, metadata, sources, stacks), mirroring the surface already exposed by `describe affected | component | stacks`. Bound to ATMOS_SKIP, with ATMOS_AFFECTED_SKIP preserved as a back-compat alias for `list affected`. Threads the value through ExecuteDescribeStacks (which already accepted skip but was being passed nil at every list callsite). Also makes three documented flags on `list instances` actually do what the docs say: --stack now filters with path.Match glob semantics (previously returned every instance); --filter evaluates a YQ predicate per row (previously a TODO stub); --query projects each row via YQ with scalars landing in a `value` column and maps flattened to row keys (previously read into options and dropped). The implementation also closes a latent ENV-precedence gap so ATMOS_LIST_FORMAT and ATMOS_UPLOAD are honored via viper instead of cobra re-reads. Includes parser, options, and propagation tests for --skip (with a regression test for the literal `list instances --upload --skip terraform.state` failure), plus unit and integration tests for the stack/filter/query work. Docs and release-blog entries for both features. Roadmap milestone added under the Discoverability initiative. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
May 15, 2026 20:48
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
![atmos-pro[bot]](https://avatars.githubusercontent.com/in/927493?s=60&v=4){kind=small}
Erik Osterman (Cloud Posse) (osterman)
added
the
patch
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a repeatable ChangesUnified List Commands Enhancement
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
cmd/list/flag_wrappers_test.go (1)
250-264: ⚡ Quick winAdd alias-isolation regression coverage for
skip.Nice coverage for registration/defaults. Add one guard test asserting
ATMOS_AFFECTED_SKIPdoes not affect non-affected commands, whileATMOS_SKIPstill does. That prevents the shared-wrapper alias leak from regressing silently.As per coding guidelines "
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages."Also applies to: 326-356, 463-464
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/list/flag_wrappers_test.go` around lines 250 - 264, Add a new unit test alongside TestWithSkipFlag that verifies alias-isolation: create a parser via NewListParser(WithSkipFlag) and a non-affected command (e.g., &cobra.Command{Use: "test"}), then set environment variables ATMOS_AFFECTED_SKIP and ATMOS_SKIP separately and assert that ATMOS_SKIP still populates the skip flag while ATMOS_AFFECTED_SKIP does NOT modify the flag for this non-affected command; use parser.RegisterFlags(cmd) and cmd.Flags().Lookup("skip") to inspect values and ensure the alias leak between the shared wrapper and affected-command alias is prevented.pkg/list/list_instances_stackfilter_test.go (1)
89-92: ⚡ Quick winAssert returned instance content, not just slice length, in stack-filter tests.
These subtests can pass with wrong rows as long as the count matches. Add explicit checks (e.g., first/last stack+component) to lock behavior.
As per coding guidelines: "For slice-result tests, assert element contents, not just length; use
require.Lencombined with assertions on at least the first and last element by value".Also applies to: 101-107
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/list/list_instances_stackfilter_test.go` around lines 89 - 92, The test for collectInstances currently only asserts slice length which can hide incorrect rows; update the subtests (e.g., the one using collectInstances(stacks, "")) to use require.Len to assert the count and then assert the actual contents of at least the first and last elements (check expected stack and component fields) so the test verifies element values as well as length; locate the tests referencing collectInstances and stacks and add assertions comparing expected struct field values for the first and last entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/list/flag_wrappers.go`:
- Around line 422-430: WithSkipFlag currently binds the legacy
ATMOS_AFFECTED_SKIP env var for all commands; remove ATMOS_AFFECTED_SKIP from
the flags.WithEnvVars call inside WithSkipFlag so it only exposes ATMOS_SKIP,
and then add the legacy ATMOS_AFFECTED_SKIP binding only where the "list
affected" command is constructed (i.e. when registering the affected subcommand
that calls WithSkipFlag) so that ATMOS_AFFECTED_SKIP remains supported solely
for the list affected command.
In `@pkg/list/list_instances.go`:
- Around line 170-184: The current matchStackPattern silently treats invalid
glob patterns as “no match”; change its behavior to validate the pattern and
surface an ErrInvalidFlag instead of returning false on path.Match errors:
update matchStackPattern(stackName, pattern) to return (bool, error) (or create
a separate validateStackPattern(pattern) that calls path.Match with
filepath.ToSlash and returns ErrInvalidFlag when err != nil), keep the
empty-pattern => true logic, return matched, nil on success, and update all
callers (including the other occurrence around lines ~205-215) to handle the
error and propagate or present ErrInvalidFlag to the user.
In `@website/src/data/roadmap.js`:
- Around line 251-252: Two shipped milestones are missing the required pr field;
add a pr: <number> property to each milestone object: the one with changelog:
'list-skip-flag' (label contains "`--skip` flag across every `atmos list`
command...") and the one with changelog: 'list-instances-stack-filter-query'
(label contains "`atmos list instances` `--stack`, `--filter`, `--query` now
work as documented"), setting the value to the PR number that implements each
changelog entry so the roadmap metadata contract is satisfied.
---
Nitpick comments:
In `@cmd/list/flag_wrappers_test.go`:
- Around line 250-264: Add a new unit test alongside TestWithSkipFlag that
verifies alias-isolation: create a parser via NewListParser(WithSkipFlag) and a
non-affected command (e.g., &cobra.Command{Use: "test"}), then set environment
variables ATMOS_AFFECTED_SKIP and ATMOS_SKIP separately and assert that
ATMOS_SKIP still populates the skip flag while ATMOS_AFFECTED_SKIP does NOT
modify the flag for this non-affected command; use parser.RegisterFlags(cmd) and
cmd.Flags().Lookup("skip") to inspect values and ensure the alias leak between
the shared wrapper and affected-command alias is prevented.
In `@pkg/list/list_instances_stackfilter_test.go`:
- Around line 89-92: The test for collectInstances currently only asserts slice
length which can hide incorrect rows; update the subtests (e.g., the one using
collectInstances(stacks, "")) to use require.Len to assert the count and then
assert the actual contents of at least the first and last elements (check
expected stack and component fields) so the test verifies element values as well
as length; locate the tests referencing collectInstances and stacks and add
assertions comparing expected struct field values for the first and last
entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7ceee590-1c72-40d6-8def-0b8a4b739f11
📒 Files selected for processing (28)
cmd/list/components.gocmd/list/flag_wrappers.gocmd/list/flag_wrappers_test.gocmd/list/instances.gocmd/list/instances_test.gocmd/list/metadata.gocmd/list/parse_options_test.gocmd/list/sources.gocmd/list/stacks.gopkg/list/filter/yq.gopkg/list/filter/yq_test.gopkg/list/list_instances.gopkg/list/list_instances_bench_test.gopkg/list/list_instances_cmd_test.gopkg/list/list_instances_comprehensive_test.gopkg/list/list_instances_coverage_test.gopkg/list/list_instances_integration_test.gopkg/list/list_instances_process_test.gopkg/list/list_instances_stackfilter_test.gopkg/list/list_metadata.gowebsite/blog/2026-05-15-list-instances-stack-filter-query.mdxwebsite/blog/2026-05-15-list-skip-flag.mdxwebsite/docs/cli/commands/list/list-components.mdxwebsite/docs/cli/commands/list/list-instances.mdxwebsite/docs/cli/commands/list/list-metadata.mdxwebsite/docs/cli/commands/list/list-sources.mdxwebsite/docs/cli/commands/list/list-stacks.mdxwebsite/src/data/roadmap.js
Erik Osterman (Cloud Posse) (osterman)
added
minor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/list/list_instances_stackfilter_test.go (1)
112-116: ⚡ Quick winStrengthen slice assertions in the “all instances” case.
Line [115] checks only length; add value assertions (at least first/last elements) so the test validates content, not just count.
As per coding guidelines "For slice-result tests, assert element contents, not just length; use
require.Lencombined with assertions on at least the first and last element by value."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/list/list_instances_stackfilter_test.go` around lines 112 - 116, The test "empty pattern returns all instances" currently only checks length; change to require.Len(t, got, 3) to fail fast and then assert the actual contents of the slice (at least the first and last elements) to validate values returned by collectInstances(stacks, ""); use the local variable got and compare got[0] and got[len(got)-1] against the expected instance identifiers/structures from your test fixture (the expected values used elsewhere in the test file) so the test verifies content, not just count.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/list/flag_wrappers_test.go`:
- Around line 466-492: The two subtests for skip behavior can flake if
ATMOS_SKIP is set in the host environment; in each t.Run (the ones creating
parser := NewListParser(WithSkipFlag) and parser :=
NewListParser(WithAffectedSkipFlag)) explicitly set ATMOS_SKIP (e.g.,
t.Setenv("ATMOS_SKIP", "")) in addition to ATMOS_AFFECTED_SKIP so tests are
isolated; keep the rest of the flow (parser.RegisterFlags, parser.BindToViper,
parser.BindFlagsToViper and assertions on v.GetStringSlice("skip")) unchanged.
In `@pkg/list/list_instances_bench_test.go`:
- Line 31: The benchmark currently ignores the error returned by
collectInstances(stacksMap, ""), which can hide regressions; change the call to
capture the error (e.g., "_, err := collectInstances(stacksMap, \"\")"), check
if err != nil, and call b.Fatalf with a clear message including err to fail the
benchmark on unexpected collection errors (reference: collectInstances,
stacksMap, and b.Fatalf).
---
Nitpick comments:
In `@pkg/list/list_instances_stackfilter_test.go`:
- Around line 112-116: The test "empty pattern returns all instances" currently
only checks length; change to require.Len(t, got, 3) to fail fast and then
assert the actual contents of the slice (at least the first and last elements)
to validate values returned by collectInstances(stacks, ""); use the local
variable got and compare got[0] and got[len(got)-1] against the expected
instance identifiers/structures from your test fixture (the expected values used
elsewhere in the test file) so the test verifies content, not just count.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36cb927e-c617-4260-824d-a0c9c74900cb
📒 Files selected for processing (11)
cmd/list/affected.gocmd/list/flag_wrappers.gocmd/list/flag_wrappers_test.gopkg/list/filter/yq.gopkg/list/list_instances.gopkg/list/list_instances_bench_test.gopkg/list/list_instances_cmd_test.gopkg/list/list_instances_comprehensive_test.gopkg/list/list_instances_integration_test.gopkg/list/list_instances_stackfilter_test.gowebsite/src/data/roadmap.js
✅ Files skipped from review due to trivial changes (1)
- website/src/data/roadmap.js
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/list/list_instances_cmd_test.go
- pkg/list/list_instances_comprehensive_test.go
- pkg/list/filter/yq.go
- pkg/list/list_instances.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2413 +/- ##
==========================================
+ Coverage 78.06% 78.12% +0.06%
==========================================
Files 1110 1111 +1
Lines 104673 104973 +300
==========================================
+ Hits 81713 82012 +299
+ Misses 18425 18412 -13
- Partials 4535 4549 +14
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
# Conflicts: # cmd/list/instances.go # pkg/list/list_instances.go
Andriy Knysh (aknysh)
approved these changes
May 16, 2026
Contributor
|
These changes were released in v1.218.1-rc.1. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
--skipacross everyatmos listsubcommand.instances,components,metadata,sources, andstacksnow accept--skip <yaml-function>(repeatable, e.g.--skip terraform.state --skip terraform.output). Mirrors the surface already exposed bydescribe affected | component | stacks. Bound toATMOS_SKIP; the existingATMOS_AFFECTED_SKIPcontinues to work onlist affectedas a back-compat alias. Threads throughExecuteDescribeStacks— which already acceptedskipbut was being passednilat every list callsite.--stack,--filter, and--queryonatmos list instancesnow work. Three documented flags were previously silent:--stackwas ignored (every instance returned),--filterwas a TODO stub, and--querywas read into options and dropped.--stacknow usespath.Matchglob semantics,--filterevaluates a YQ predicate per row, and--queryprojects each row via YQ (scalars land in avaluecolumn, maps flatten to row keys). Closes a latent ENV-precedence gap soATMOS_LIST_FORMATandATMOS_UPLOADare honored via viper.--skip(with a regression test for the literallist instances --upload --skip terraform.statefailure). Unit + integration tests for the stack/filter/query work. Newpkg/list/filter/yq.go(YQPredicateFilter,YQProjector,isTruthy) with full coverage.--skipdocumented on all five list pages; two release-blog entries; one roadmap milestone under the Discoverability initiative.why
--skip:atmos list instances --upload --skip terraform.stateerrored withunknown flag.--process-functions=falseis not a substitute because it also disables!template, which Atmos Pro uploads need sosettings.pro.enabledevaluates to a real boolean instead of a literal string.--stack/--filter/--query: the docs promised filtering onlist instancesand the implementation didn't honor it. Users hit silent wrong-result behavior, not an error.listfiles; bundling avoids merge churn and keeps the test+docs surface coherent.references
--skiprollout: feat(list): --process-templates and --process-functions flags; fix list instances --upload auth #2363 (--process-templates/--process-functionsrollout acrosslist)--skip(describe family): Addprocess-templates,--process-functionsand--skipflags toatmos describe affected,atmos describe componentandatmos describe stackscommands #1006Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Tests
Documentation