Repository navigation
feat(list): add matrix output format to list instances command - #2322
Merged
Merged
Conversation
Add --format=matrix and --output-file support to `atmos list instances`, producing GitHub Actions-compatible matrix JSON identical to `atmos describe affected --format=matrix`. Extract shared matrix types and output logic into pkg/matrix/ for DRY reuse across both commands.
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
Copilot started reviewing on behalf of
John C. Bland II (johncblandii)
April 13, 2026 19:01
View session
Dependency Review✅ No vulnerabilities or license issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
John C. Bland II (johncblandii)
added
the
minor
|
Warning Release Documentation RequiredThis PR is labeled
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a shared GitHub Actions matrix output implementation and wires it into atmos list instances, aligning behavior with the existing describe affected --format=matrix flow and enabling $GITHUB_OUTPUT-style file output.
Changes:
- Introduces
pkg/matrixwith shared matrix types plus JSON andkey=valueoutput writing. - Adds
--format=matrixand--output-fileplumbing toatmos list instances. - Refactors
describe affectedmatrix output to use the sharedpkg/matriximplementation and updates/relocates related tests.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/matrix/matrix.go | New shared matrix types + marshal/write helpers. |
| pkg/matrix/matrix_test.go | Unit tests for matrix marshaling and output writing. |
| pkg/list/list_instances.go | Adds matrix-format execution path and output-file support. |
| pkg/list/list_instances_coverage_test.go | Coverage tests for list-instances matrix mode and output-file writing. |
| pkg/list/format/formatter.go | Adds matrix to format constants/validation. |
| pkg/list/format/formatter_test.go | Updates validation tests to include matrix. |
| pkg/list/extract/matrix.go | New helper to extract stack/component entries into matrix entries. |
| pkg/list/extract/matrix_test.go | Tests for stacksMap → matrix entry extraction. |
| internal/exec/describe_affected.go | Switches matrix output to pkg/matrix and converts to []matrix.Entry. |
| internal/exec/describe_affected_test.go | Updates affected→matrix conversion tests and removes duplicated matrix output tests. |
| cmd/list/instances.go | Adds --output-file flag wiring and passes through to pkg/list options. |
| cmd/list/instances_test.go | Extends options tests and checks output-file flag registration. |
| cmd/list/flag_wrappers.go | Extends shared --format valid values and adds --output-file wrapper. |
| cmd/list/stacks.go | Trailing newline change only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a GitHub Actions matrix output mode to Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as CLI Parser
participant List as ExecuteListInstancesCmd
participant Describe as ExecuteDescribeStacks
participant Extract as extract.StacksMatrixEntries
participant Matrix as pkg/matrix.WriteOutput
participant File as File/Stdout
User->>CLI: atmos list instances --format=matrix --output-file=out.txt
CLI->>List: run with opts(format=matrix, outputFile=out.txt)
List->>Describe: ExecuteDescribeStacks()
Describe-->>List: stacksMap
List->>Extract: StacksMatrixEntries(stacksMap)
Extract-->>List: []matrix.Entry
List->>Matrix: WriteOutput(entries, outputFile)
alt outputFile set
Matrix->>File: append "matrix=<json>\ncount=<n>\n"
File-->>Matrix: written
else
Matrix->>File: write JSON to stdout
File-->>Matrix: written
end
Matrix-->>List: result
List-->>User: exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
pkg/matrix/matrix.go (2)
57-59: Consider handling the write error.The error from
data.Writelnis discarded. If stdout write fails, the caller won't know.Suggested fix
// Write to stdout. - _ = data.Writeln(string(matrixJSON)) - return nil + if err := data.Writeln(string(matrixJSON)); err != nil { + return fmt.Errorf("failed to write matrix output: %w", err) + } + return nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/matrix/matrix.go` around lines 57 - 59, The call to data.Writeln(string(matrixJSON)) ignores its error; update the code in the function that writes matrixJSON (the data.Writeln call) to capture the returned error, and if non-nil return it (or wrap it with context, e.g., fmt.Errorf("writing matrix JSON to stdout: %w", err)) instead of discarding it so the caller can detect stdout write failures.
66-81: Close error should be checked for write operations.When writing to files,
Close()can return errors indicating data wasn't flushed. Usingdeferalone discards this.Suggested fix
func writeToFile(matrixJSON []byte, count int, outputFile string) error { defer perf.Track(nil, "matrix.writeToFile")() f, err := os.OpenFile(outputFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, defaultFilePermissions) if err != nil { return fmt.Errorf("failed to open output file %s: %w", outputFile, err) } - defer f.Close() // Write matrix=<json> format. if _, err := fmt.Fprintf(f, "matrix=%s\n", string(matrixJSON)); err != nil { + _ = f.Close() return fmt.Errorf("failed to write to output file %s: %w", outputFile, err) } // Also write count for convenience. if _, err := fmt.Fprintf(f, "affected_count=%d\n", count); err != nil { + _ = f.Close() return fmt.Errorf("failed to write count to output file %s: %w", outputFile, err) } log.Debug("Wrote matrix output to file", "file", outputFile, "count", count) - return nil + return f.Close() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/matrix/matrix.go` around lines 66 - 81, The deferred f.Close() call currently ignores Close errors; change the function that opens outputFile (the scope with f, matrixJSON, count) to use a named error return (e.g., err error) and replace defer f.Close() with a defer that captures the Close error: defer func() { if cerr := f.Close(); cerr != nil && err == nil { err = fmt.Errorf("failed to close output file %s: %w", outputFile, cerr) } }(), so any flush/close failure is returned (and preserves earlier write errors if present) after the fmt.Fprintf calls that write "matrix=" and "affected_count=" to f. Ensure you reference the file handle f, outputFile and the named err return when implementing this change.pkg/list/extract/matrix.go (1)
82-140: Consider extracting shared iteration logic.
StacksMatrixEntriesandStacksMatrixEntriesForComponentshare ~80% of their code. A helper could reduce duplication:func iterateStacks(stacksMap map[string]any, componentFilter string, fn func(stackName, componentName, componentType string, componentData map[string]any))This is optional - the current code is readable and works correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/list/extract/matrix.go` around lines 82 - 140, Extract the shared stack/component iteration into a helper (e.g., iterateStacks) so both StacksMatrixEntries and StacksMatrixEntriesForComponent reuse it: implement iterateStacks(stacksMap map[string]any, componentFilter string, fn func(stackName, componentName, componentType string, componentData map[string]any)) that performs the sorted stackName loop, type/component lookups (using getComponentTypes), and invokes fn for each matching component; then replace the duplicated loops in StacksMatrixEntries and StacksMatrixEntriesForComponent with calls to iterateStacks and move matrix.Entry construction and component_info/component_path extraction into the per-item callback to populate entries.cmd/list/flag_wrappers.go (1)
42-43: Comment mentions wrong command.The comment says "Used by: stacks" but per the PR context, this flag is used by the
instancescommand with--format=matrix.📝 Suggested fix
// WithOutputFileFlag adds output file flag for writing results in key=value format (for $GITHUB_OUTPUT). -// Used by: stacks (with --format=matrix). +// Used by: instances (with --format=matrix). func WithOutputFileFlag(options *[]flags.Option) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/list/flag_wrappers.go` around lines 42 - 43, Update the comment on WithOutputFileFlag in flag_wrappers.go to reference the correct command: change "Used by: stacks (with --format=matrix)" to "Used by: instances (with --format=matrix)" so the documentation matches actual usage of the WithOutputFileFlag helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/list/flag_wrappers.go`:
- Around line 42-43: Update the comment on WithOutputFileFlag in
flag_wrappers.go to reference the correct command: change "Used by: stacks (with
--format=matrix)" to "Used by: instances (with --format=matrix)" so the
documentation matches actual usage of the WithOutputFileFlag helper.
In `@pkg/list/extract/matrix.go`:
- Around line 82-140: Extract the shared stack/component iteration into a helper
(e.g., iterateStacks) so both StacksMatrixEntries and
StacksMatrixEntriesForComponent reuse it: implement iterateStacks(stacksMap
map[string]any, componentFilter string, fn func(stackName, componentName,
componentType string, componentData map[string]any)) that performs the sorted
stackName loop, type/component lookups (using getComponentTypes), and invokes fn
for each matching component; then replace the duplicated loops in
StacksMatrixEntries and StacksMatrixEntriesForComponent with calls to
iterateStacks and move matrix.Entry construction and
component_info/component_path extraction into the per-item callback to populate
entries.
In `@pkg/matrix/matrix.go`:
- Around line 57-59: The call to data.Writeln(string(matrixJSON)) ignores its
error; update the code in the function that writes matrixJSON (the data.Writeln
call) to capture the returned error, and if non-nil return it (or wrap it with
context, e.g., fmt.Errorf("writing matrix JSON to stdout: %w", err)) instead of
discarding it so the caller can detect stdout write failures.
- Around line 66-81: The deferred f.Close() call currently ignores Close errors;
change the function that opens outputFile (the scope with f, matrixJSON, count)
to use a named error return (e.g., err error) and replace defer f.Close() with a
defer that captures the Close error: defer func() { if cerr := f.Close(); cerr
!= nil && err == nil { err = fmt.Errorf("failed to close output file %s: %w",
outputFile, cerr) } }(), so any flush/close failure is returned (and preserves
earlier write errors if present) after the fmt.Fprintf calls that write
"matrix=" and "affected_count=" to f. Ensure you reference the file handle f,
outputFile and the named err return when implementing this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a810297-b713-422f-bc8f-9ec54a929faf
📒 Files selected for processing (14)
cmd/list/flag_wrappers.gocmd/list/instances.gocmd/list/instances_test.gocmd/list/stacks.gointernal/exec/describe_affected.gointernal/exec/describe_affected_test.gopkg/list/extract/matrix.gopkg/list/extract/matrix_test.gopkg/list/format/formatter.gopkg/list/format/formatter_test.gopkg/list/list_instances.gopkg/list/list_instances_coverage_test.gopkg/matrix/matrix.gopkg/matrix/matrix_test.go
…atrix output Fix PR review comments: propagate data.Writeln error in WriteOutput, normalize nil entries to empty slice in Marshal, rename env var to ATMOS_LIST_OUTPUT_FILE, scope ValidateFormat to renderer-compatible formats only (tree/matrix are special-cased by commands). Add CI auto-detection: when ci.enabled is true and no --output-file is provided, automatically write matrix output to $GITHUB_OUTPUT.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/list/list_instances.go`:
- Around line 415-421: Add a guard that returns an explicit invalid-flag error
when --output-file is provided for non-matrix formats: inside the same branch
that handles the matrix format check (using formatFlag and format.FormatMatrix,
near the executeMatrixFormat(&atmosConfig, opts) call), detect if an outputFile/
output-file flag is set and formatFlag != string(format.FormatMatrix) and return
fmt.Errorf("%w: --output-file is only supported with --format=matrix",
errUtils.ErrInvalidFlag); this prevents silently ignoring --output-file for
other formats.
🪄 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: e68fdd00-c3a8-483f-934f-86082279a5c3
📒 Files selected for processing (6)
cmd/list/flag_wrappers.gopkg/list/format/formatter.gopkg/list/format/formatter_test.gopkg/list/list_instances.gopkg/matrix/matrix.gopkg/matrix/matrix_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/matrix/matrix.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/list/format/formatter.go
- cmd/list/flag_wrappers.go
- pkg/matrix/matrix_test.go
- pkg/list/format/formatter_test.go
Add guard in both list instances and describe affected to return an explicit ErrInvalidFlag when --output-file is provided with a format other than matrix. Previously the flag was silently ignored.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2322 +/- ##
==========================================
+ Coverage 77.23% 77.27% +0.04%
==========================================
Files 1074 1076 +2
Lines 101910 101999 +89
==========================================
+ Hits 78707 78824 +117
+ Misses 18860 18828 -32
- Partials 4343 4347 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Drop unused StacksMatrixEntriesForComponent and its tests - Rename output key from affected_count to count (shared across describe affected and list instances) - Document --format=matrix and --output-file on the list instances page - Refactor StacksMatrixEntries into smaller helpers to reduce cognitive complexity Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/exec/describe_affected_test.go (1)
1604-1607: Assert full entries in the multi-item case.This only proves ordering of two strings. A regression that drops or corrupts
ComponentPath/ComponentTypefor later items would still pass, so it would be better to compare the first and lastmatrix.Entryvalues directly. As per coding guidelines, "For slice-result tests, assert element contents, not just length.require.Lenalone allows regressions that drop or corrupt contents. Assert at least the first and last element by value."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/exec/describe_affected_test.go` around lines 1604 - 1607, The test currently only asserts length and two individual string fields; instead assert the full first and last matrix.Entry values to prevent silent regressions—locate where convertAffectedToMatrix is called (entries := convertAffectedToMatrix(affected)) and replace the weak checks with equality assertions that compare entries[0] and entries[len(entries)-1] against expected matrix.Entry instances (including Stack, Component, ComponentPath/ComponentType and any other fields) so the test verifies element contents, not just ordering or length.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/matrix/matrix.go`:
- Around line 51-63: Replace the ad-hoc dynamic errors returned from
Marshal(entries), writeToFile(...), and data.Writeln(...) with wrapped errors
that use the repository sentinel errors defined in errors/errors.go: capture
each underlying error (err) and return fmt.Errorf("<context>: %w",
errors.ErrSomething) where ErrSomething is the appropriate sentinel (e.g.,
ErrMarshal, ErrWriteFile, ErrWriteStdout — pick the exact names from
errors/errors.go) and wrap the original err using %w so callers can errors.Is;
if multiple failures must be combined use errors.Join to attach both the
sentinel and the original error; apply the same wrapping pattern to the later
branch (lines 71–84) as well.
In `@website/docs/cli/commands/list/list-instances.mdx`:
- Line 201: The sentence overstates behavior: update the docs for list-instances
(referencing `ci.enabled`, `--output-file`) to indicate that automatic writing
to `$GITHUB_OUTPUT` only occurs when a GitHub Actions output path is available
(e.g., `GITHUB_OUTPUT` env var present); tighten wording to say "When
`ci.enabled` is true in `atmos.yaml` and a GitHub Actions output path is
available (e.g., `GITHUB_OUTPUT` is set) and `--output-file` is not provided,
output is written to `$GITHUB_OUTPUT`; otherwise output falls back to stdout."
Ensure the doc mentions both cases and keeps CLI and website docs consistent.
---
Nitpick comments:
In `@internal/exec/describe_affected_test.go`:
- Around line 1604-1607: The test currently only asserts length and two
individual string fields; instead assert the full first and last matrix.Entry
values to prevent silent regressions—locate where convertAffectedToMatrix is
called (entries := convertAffectedToMatrix(affected)) and replace the weak
checks with equality assertions that compare entries[0] and
entries[len(entries)-1] against expected matrix.Entry instances (including
Stack, Component, ComponentPath/ComponentType and any other fields) so the test
verifies element contents, not just ordering or length.
🪄 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: 07f9d455-3f33-4da0-895c-4ef199c73bad
📒 Files selected for processing (8)
internal/exec/describe_affected_test.gopkg/list/extract/matrix.gopkg/list/extract/matrix_test.gopkg/list/list_instances_coverage_test.gopkg/matrix/matrix.gopkg/matrix/matrix_test.gowebsite/blog/2026-04-13-list-instances-matrix.mdxwebsite/docs/cli/commands/list/list-instances.mdx
✅ Files skipped from review due to trivial changes (1)
- pkg/matrix/matrix_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/list/list_instances_coverage_test.go
…sertions - Wrap pkg/matrix errors with static sentinels (ErrFailedToMarshalPayload, ErrWriteOutput, ErrOpenFile, ErrWriteFile) for errors.Is() matching - Assert full matrix.Entry values in multi-item convertAffectedToMatrix test - Clarify docs: auto-redirect to GITHUB_OUTPUT requires both ci.enabled and GITHUB_OUTPUT env var Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Andriy Knysh (aknysh)
approved these changes
Apr 14, 2026
Contributor
|
These changes were released in v1.216.0-rc.0. |
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Aug 3, 2026
These nine carried `authors: [atmos]`, the generic team byline the changelog skill says to avoid. Attribution follows the pull request that implemented the feature, not the commit that added or later renamed the post file. That distinction mattered. Five of the nine appear in `git log --diff-filter=A` as added by #2753, a chronology-correction pass that renamed post files to match their publication dates. Following renames instead points at the real work: list-components-fix #1949 osterman chdir-config-isolation #1941 osterman introducing-atmos-lsp #2030 aknysh introducing-atmos-ai #2030 aknysh ci-comments-env-var #2300 osterman list-instances-matrix #2322 johncblandii terraform-all-dependency-order-wired-up #2486 thejrose1984 dotenv-include-support #1930 osterman toolchain-proxies #1687 osterman Each byline is the GitHub pull request author rather than the git commit author, because squash merges attribute the commit to whoever merged it. #2300 needed a judgment call. It was authored by `app/copilot-swe-agent`, a bot. The byline goes to osterman, who is the assignee and the human who drove the change. A bot is not a contributor byline, and the person who merged it (aknysh) did not do the work either. johncblandii and thejrose1984 were missing from `authors.yml` and are added here, which the changelog skill requires in the same change that references them. thejrose1984 publishes no display name on GitHub, so the login stands in rather than inventing one. Verified: all 263 posts now resolve to a real `authors.yml` entry, no post carries the generic byline, and the site builds with no author warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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
--format=matrixsupport toatmos list instances, producing GitHub Actions-compatible matrix JSON identical toatmos describe affected --format=matrix--output-fileflag for writing results inkey=valueformat (for$GITHUB_OUTPUT)pkg/matrix/for DRY reuse across bothdescribe affectedandlist instanceswhy
list instancesto drive parallel GitHub Actions jobs, just likedescribe affectedalready supports--output-fileflag enables direct integration with GitHub Actions$GITHUB_OUTPUTwithout shell redirectionreferences
atmos describe affected --format=matrixexactly:{"include":[{"stack":"...","component":"...","component_path":"...","component_type":"..."}]}--output-file, writesmatrix=<json>andaffected_count=<N>linesSummary by CodeRabbit
New Features
Tests
Documentation