Repository navigation
feat: format-preserving YAML edits + Atmos Version Tracker - #2664
Merged
Merged
Conversation
…vendor Add a shared pkg/yaml editing engine built on yqlib (fed raw bytes) that preserves comments, anchors/aliases, Atmos YAML functions, and Go templates, with a strict guard that rejects edits which would alter or expand an anchor. Expose it via three dot-notation command groups: - atmos config get|set|delete (edits the active atmos.yaml) - atmos stack get|set|delete (provenance resolves the manifest that defines the effective value; --file override) - atmos vendor get|set (version pinning by component name) Includes a PRD, Docusaurus docs, a changelog blog post, and a roadmap milestone. Supersedes the hardcoded sources[].version writer from the old feat/vendor-diff-and-update branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
June 28, 2026 03:55
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
Dependency Review✅ No vulnerabilities or license issues found.Scanned Files
|
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 format-preserving YAML editing for config, stack, and vendor flows. The PR adds a shared YAML engine, typed value handling, provenance-aware stack target resolution, vendor update/diff helpers, new CLI commands, and matching docs and tests. ChangesFormat-Preserving YAML Editing Feature
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90 minutes Suggested labels
Suggested reviewers
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
website/docs/cli/commands/stack/stack-delete.mdx (2)
10-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the edit limitations on the delete page too.
This page describes in-place edits, but it omits the same anchor/alias rejection and blank-line-loss caveats already called out for the other edit commands in this feature. That leaves the delete contract understated for users.
🤖 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 `@website/docs/cli/commands/stack/stack-delete.mdx` around lines 10 - 48, Update the stack delete docs to also mention the edit limitations already documented for the other stack edit commands: this page should explicitly call out that anchored/aliased values are rejected and that deleting an item may collapse blank-line spacing in the manifest. Add this note in the existing delete command content near the Intro/Flags section in stack-delete.mdx, keeping the wording consistent with the other edit command docs.
1-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the required Screengrab section.
This new CLI command page skips the mandatory Screengrab block, so it does not meet the docs contract for
website/docs/cli/commands/**/*.mdx. As per coding guidelines, "CLI command docs MUST include: (1) Frontmatter, (2) Intro component, (3) Screengrab, (4) Usage section, (5) Arguments/Flags with
- /
- , (6) Examples section."
🤖 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 `@website/docs/cli/commands/stack/stack-delete.mdx` around lines 1 - 48, The stack delete CLI doc is missing the required Screengrab block, so update the `stack-delete.mdx` page to include a Screengrab section in the standard CLI command docs layout. Place it after the `Intro` component and before `Usage`, matching the pattern used by other command pages, and keep the existing `Usage`, `Arguments`, `Flags`, and `Examples` sections intact.Source: Coding guidelines
website/docs/cli/commands/stack/stack-get.mdx (1)
1-43: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the required Screengrab section.
This new CLI command page skips the mandatory Screengrab block, so it does not meet the docs contract for
website/docs/cli/commands/**/*.mdx. As per coding guidelines, "CLI command docs MUST include: (1) Frontmatter, (2) Intro component, (3) Screengrab, (4) Usage section, (5) Arguments/Flags with
- /
- , (6) Examples section."
🤖 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 `@website/docs/cli/commands/stack/stack-get.mdx` around lines 1 - 43, The stack-get CLI doc is missing the required Screengrab section, so update the command page to include it in the standard docs structure. Add the Screengrab block to this MDX alongside the existing Frontmatter, Intro, Usage, Arguments, Flags, and Examples, following the same pattern used by other CLI command pages under the command docs. Ensure the new section is placed in the document flow where readers expect a visual command screenshot.Source: Coding guidelines
🧹 Nitpick comments (5)
pkg/config/config_edit_test.go (1)
48-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a repo-root fallback test.
The suite never exercises the
ProcessTagGitRoot("!repo-root .")branch, so the final precedence step inResolveEditableConfigFilecan regress unnoticed.pkg/utils/git.goalready exposesTEST_GIT_ROOTspecifically for test isolation, so this is easy to cover. As per coding guidelines, “Maintain 80% minimum test coverage (CodeCov enforced). All features need tests.”🤖 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/config/config_edit_test.go` around lines 48 - 64, Add a test that covers the repo-root fallback path in ResolveEditableConfigFile, since the current config_edit tests only verify the current-directory case and miss the ProcessTagGitRoot("!repo-root .") branch. Use the TEST_GIT_ROOT override from pkg/utils/git.go to isolate the repo root in the test, then assert ResolveEditableConfigFile selects the repo-root config when no higher-precedence source is available. Keep the new test alongside TestResolveEditableConfigFile_CurrentDirectory so the final precedence behavior stays covered.Source: Coding guidelines
pkg/vendoring/edit_test.go (1)
74-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a non-not-found failure case for
SetComponentVersion.Once the guard only rewrites the missing-component sentinel, keep it that way with a test for malformed YAML or an unreadable file and assert the error is not reported as “component not found”. As per coding guidelines, “Include negative-path tests for recovery logic.”
🤖 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/vendoring/edit_test.go` around lines 74 - 78, Add a negative-path test for SetComponentVersion beyond the existing not-found case by exercising a malformed YAML or unreadable file scenario in edit_test.go. Use SetComponentVersion with a bad vendor file fixture and assert the returned error is not treated as the missing-component sentinel, so the recovery logic in SetComponentVersion only rewrites the “component not found” case and leaves other failures untouched.Source: Coding guidelines
cmd/vendor/edit.go (1)
68-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute
--filethrough the standard flags parser.This new command-specific flag is registered directly on Cobra. Please wire it through
flags.NewStandardParser()to match the repo’s command flag handling. As per coding guidelines, "Commands MUST useflags.NewStandardParser()for command-specific flags."🤖 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/vendor/edit.go` around lines 68 - 74, The vendor command-specific --file flag is being registered directly on Cobra instead of through the repo’s standard flag handling. Update the init setup for vendorGetCmd and vendorSetCmd in edit.go to route this flag through flags.NewStandardParser(), matching the existing command flag pattern used elsewhere in the repo. Keep the flag behavior the same, but ensure the parser wiring is done via the standard parser for both commands.Source: Coding guidelines
cmd/config/operations.go (1)
86-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute
--typethrough the standard flags parser.This registers a command-specific flag directly on Cobra. Please wire it through
flags.NewStandardParser()so the new command follows the same parsing path as the rest ofcmd/**. As per coding guidelines, "Commands MUST useflags.NewStandardParser()for command-specific flags."🤖 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/config/operations.go` around lines 86 - 89, The `configSetCmd` flag setup is registering `--type` directly on Cobra instead of using the shared parsing path. Update the flag wiring in `init()` for `configSetCmd` to route the command-specific flag through `flags.NewStandardParser()`, following the same pattern used by other `cmd/**` commands and keeping the `valueType` binding intact.Source: Coding guidelines
cmd/stack/operations.go (1)
104-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute these subcommand flags through the standard parser.
--stack,--component,--file, and--typeare all registered directly on Cobra here. Please move them ontoflags.NewStandardParser()so this command group follows the repo’s CLI flag contract. As per coding guidelines, "Commands MUST useflags.NewStandardParser()for command-specific flags."🤖 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/stack/operations.go` around lines 104 - 114, The stack subcommands are registering command-specific flags directly on Cobra, which bypasses the repo’s standard CLI contract. Move the `--stack`, `--component`, `--file`, and `--type` definitions from `init()` in `stackGetCmd`, `stackSetCmd`, and `stackDeleteCmd` to `flags.NewStandardParser()`, and wire those parsed values into the commands there so the standard parser owns all command flags. Keep the required-flag handling aligned with the standard parser’s behavior for these symbols.Source: Coding guidelines
🤖 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/stack/operations.go`:
- Around line 119-143: The shared resolveEditTarget flow is mixing read-only
lookup with edit-target validation, so stack get can inherit mutation-only
checks and ignore an explicit --file. Split the read path from
provenance/editability resolution: keep resolveEditTarget for edit actions, and
add a read-specific path in resolveEditTarget and the stack get flow that
returns the effective value from the explicit file or merged component data
without calling resolveTargetByProvenance. Make sure the change is centered
around resolveEditTarget, resolveTargetByProvenance, and the stack get handling
so valid read calls no longer fail on editability.
In `@pkg/config/config_edit.go`:
- Around line 56-78: The config path resolution logic is treating every os.Stat
failure as ErrNoEditableConfig, which hides permission and I/O problems. Update
resolveOverridePath and firstExistingConfig to only map os.IsNotExist to
“missing” and return other stat errors with context, so explicit --config, CWD,
and git-root probes don’t silently skip broken candidates. Keep the fix
localized around resolveOverridePath, firstExistingConfig, and
ErrNoEditableConfig handling.
In `@pkg/stack/edit.go`:
- Around line 22-26: The path builder in the stack edit helper is joining raw
segments with dots, so component names containing dots or bracket syntax are
parsed as nested YAML paths instead of literal keys. Update the path
construction logic in the function that builds the stack path (using
ComponentsSectionName, componentType, componentName, and relPath) to escape or
quote each segment before joining, so stack get/set/delete targets the intended
manifest node.
In `@pkg/vendoring/edit.go`:
- Around line 45-49: The component existence check in GetComponentVersion should
only translate the path-not-found case into “component not found,” not every
failure. Update the error handling in edit.go’s vendor set flow to use
errors.Is() against the YAML path-not-found sentinel, and keep unreadable-file
or invalid-YAML errors wrapped/returned unchanged so atmos vendor set reports
the real cause.
In `@pkg/yaml/edit.go`:
- Around line 92-95: `Query` in `edit.go` is conflating a real empty YAML scalar
with a missing match by checking `trimmed == ""` after `strings.TrimRight`.
Update the `trimmed` handling so only an actual no-output result is treated as
`ErrYAMLPathNotFound`, while an explicit empty string from `UnwrapScalar` is
still returned as a valid value; use the pre-trimmed `result` to detect missing
output and keep `trimmed` only for newline cleanup.
- Around line 301-329: The atomic write path in atomicWrite still relies on
os.Rename, which breaks replacing existing files on Windows. Update atomicWrite
in pkg/yaml/edit.go to use the shared pkg/filesystem.WriteFileAtomic helper
instead of manually creating, chmod-ing, and renaming the temp file. Keep the
existing permission handling behavior by passing the intended mode through the
shared helper, and add a regression test around the YAML edit flow that edits an
already-existing file to verify it is replaced correctly.
In `@pkg/yaml/path.go`:
- Around line 84-125: splitDotPath is currently skipping empty segments, which
lets malformed paths like consecutive or trailing dots be normalized instead of
rejected. Update splitDotPath to detect when a separator in the path produces an
empty key segment and return ErrInvalidYAMLExpression immediately rather than
flushing nothing; keep the existing scanQuotedSegment and scanIndexSegment
handling, but make sure flushKey and the dot-separator logic in splitDotPath
fail on empty segments so Set/Delete cannot target the wrong key.
In `@website/docs/cli/commands/config/config-set.mdx`:
- Around line 1-63: This CLI command doc is missing the required Screengrab
section mandated for `website/docs/cli/commands/**/*.mdx`. Add the Screengrab
block in the standard place between the `Intro` component and the `Usage`
section, matching the format used by other CLI command pages, while keeping the
existing frontmatter, `Intro`, `Usage`, `Arguments`, `Flags`, and `Examples`
sections intact.
In `@website/docs/cli/commands/stack/stack-set.mdx`:
- Around line 1-60: The stack-set command doc is missing the required Screengrab
section, which is mandatory for CLI command pages. Update the `stack-set.mdx`
content to include a Screengrab block in the expected CLI docs format, keeping
the existing frontmatter, `Intro`, `Usage`, `Arguments`, `Flags`, and `Examples`
sections intact.
---
Outside diff comments:
In `@website/docs/cli/commands/stack/stack-delete.mdx`:
- Around line 10-48: Update the stack delete docs to also mention the edit
limitations already documented for the other stack edit commands: this page
should explicitly call out that anchored/aliased values are rejected and that
deleting an item may collapse blank-line spacing in the manifest. Add this note
in the existing delete command content near the Intro/Flags section in
stack-delete.mdx, keeping the wording consistent with the other edit command
docs.
- Around line 1-48: The stack delete CLI doc is missing the required Screengrab
block, so update the `stack-delete.mdx` page to include a Screengrab section in
the standard CLI command docs layout. Place it after the `Intro` component and
before `Usage`, matching the pattern used by other command pages, and keep the
existing `Usage`, `Arguments`, `Flags`, and `Examples` sections intact.
In `@website/docs/cli/commands/stack/stack-get.mdx`:
- Around line 1-43: The stack-get CLI doc is missing the required Screengrab
section, so update the command page to include it in the standard docs
structure. Add the Screengrab block to this MDX alongside the existing
Frontmatter, Intro, Usage, Arguments, Flags, and Examples, following the same
pattern used by other CLI command pages under the command docs. Ensure the new
section is placed in the document flow where readers expect a visual command
screenshot.
---
Nitpick comments:
In `@cmd/config/operations.go`:
- Around line 86-89: The `configSetCmd` flag setup is registering `--type`
directly on Cobra instead of using the shared parsing path. Update the flag
wiring in `init()` for `configSetCmd` to route the command-specific flag through
`flags.NewStandardParser()`, following the same pattern used by other `cmd/**`
commands and keeping the `valueType` binding intact.
In `@cmd/stack/operations.go`:
- Around line 104-114: The stack subcommands are registering command-specific
flags directly on Cobra, which bypasses the repo’s standard CLI contract. Move
the `--stack`, `--component`, `--file`, and `--type` definitions from `init()`
in `stackGetCmd`, `stackSetCmd`, and `stackDeleteCmd` to
`flags.NewStandardParser()`, and wire those parsed values into the commands
there so the standard parser owns all command flags. Keep the required-flag
handling aligned with the standard parser’s behavior for these symbols.
In `@cmd/vendor/edit.go`:
- Around line 68-74: The vendor command-specific --file flag is being registered
directly on Cobra instead of through the repo’s standard flag handling. Update
the init setup for vendorGetCmd and vendorSetCmd in edit.go to route this flag
through flags.NewStandardParser(), matching the existing command flag pattern
used elsewhere in the repo. Keep the flag behavior the same, but ensure the
parser wiring is done via the standard parser for both commands.
In `@pkg/config/config_edit_test.go`:
- Around line 48-64: Add a test that covers the repo-root fallback path in
ResolveEditableConfigFile, since the current config_edit tests only verify the
current-directory case and miss the ProcessTagGitRoot("!repo-root .") branch.
Use the TEST_GIT_ROOT override from pkg/utils/git.go to isolate the repo root in
the test, then assert ResolveEditableConfigFile selects the repo-root config
when no higher-precedence source is available. Keep the new test alongside
TestResolveEditableConfigFile_CurrentDirectory so the final precedence behavior
stays covered.
In `@pkg/vendoring/edit_test.go`:
- Around line 74-78: Add a negative-path test for SetComponentVersion beyond the
existing not-found case by exercising a malformed YAML or unreadable file
scenario in edit_test.go. Use SetComponentVersion with a bad vendor file fixture
and assert the returned error is not treated as the missing-component sentinel,
so the recovery logic in SetComponentVersion only rewrites the “component not
found” case and leaves other failures untouched.
🪄 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: e718642a-6310-4fd7-a859-3e88d5343f5b
📒 Files selected for processing (35)
cmd/config/config.gocmd/config/operations.gocmd/root.gocmd/stack/operations.gocmd/stack/stack.gocmd/vendor/edit.godocs/prd/yaml-editing-get-set.mdpkg/config/config_edit.gopkg/config/config_edit_test.gopkg/stack/edit.gopkg/stack/edit_test.gopkg/vendoring/edit.gopkg/vendoring/edit_test.gopkg/yaml/anchors.gopkg/yaml/edit.gopkg/yaml/edit_test.gopkg/yaml/errors.gopkg/yaml/functions_templates_test.gopkg/yaml/path.gopkg/yaml/stability_test.gopkg/yaml/typed.gowebsite/blog/2026-06-27-yaml-editing-config-stack-vendor.mdxwebsite/docs/cli/commands/config/_category_.jsonwebsite/docs/cli/commands/config/config-delete.mdxwebsite/docs/cli/commands/config/config-get.mdxwebsite/docs/cli/commands/config/config-set.mdxwebsite/docs/cli/commands/config/usage.mdxwebsite/docs/cli/commands/stack/_category_.jsonwebsite/docs/cli/commands/stack/stack-delete.mdxwebsite/docs/cli/commands/stack/stack-get.mdxwebsite/docs/cli/commands/stack/stack-set.mdxwebsite/docs/cli/commands/stack/usage.mdxwebsite/docs/cli/commands/vendor/vendor-get.mdxwebsite/docs/cli/commands/vendor/vendor-set.mdxwebsite/src/data/roadmap.js
The new top-level `config` and `stack` command groups appear in `atmos --help`, `help`, and the unknown-command list. Regenerate the affected golden snapshots via -regenerate-snapshots (content-only change, identical across linux/macos/windows). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…serving engine
atmos vendor update checks each Git-backed source for a newer allowed version
(honoring per-source semver constraints, exclusions, and no_prereleases) and
updates the version field in place via the shared pkg/yaml engine — preserving
comments, anchors, and {{.Version}} templates. Supports --check (dry run),
--pull, --component, --tags, and --outdated.
atmos vendor diff shows the Git diff between two versions of a vendored
component without a local checkout.
Both use go-git (no git binary) behind mockable interfaces; tag listing and
ref-to-ref diff are network-isolated in tests. Adds a constraints schema to
AtmosVendorSource, a PRD, command docs, and reuses the existing
Masterminds/semver dependency. Supersedes the YAML-write approach of the old
feat/vendor-diff-and-update branch (~7k lines) with ~1k lines on the shared
engine.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the actionable CodeRabbit findings on the YAML get/set/delete PR: - yaml.Query: return a legitimate empty-string scalar instead of treating it as not-found (detect missing from the pre-trimmed result). - yaml.atomicWrite: use the shared cross-platform filesystem.WriteFileAtomic so an existing file can be replaced on Windows; preserve existing mode. - yaml.splitDotPath: reject empty path segments (a..b, trailing a.) so a typo cannot silently target a different key; refactor into dotPathScanner. - config: only os.IsNotExist maps to "no editable config"; propagate other stat errors with context (firstExistingConfig now returns an error). - vendoring.SetComponentVersion: gate the "component not found" rewrite on errors.Is(ErrYAMLPathNotFound); surface unreadable/invalid-YAML causes. - stack get: split the read path from the editable resolver (requireEditable) so get honors --file and does not fail on inherited/imported values. - stack edit: escape component-name path segments (BuildComponentYqPath + yaml.QuotePathSegment) so dotted/bracketed names address the right node, while keeping the raw path for provenance lookups. - docs: add the mandatory Screengrab block to all new config/stack/vendor command pages and register their --help screengrabs in demo-stacks.txt. Add regression tests for each fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
size/xl
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
pkg/vendoring/version/version_test.go (1)
81-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the compile-time guard for
schema.VendorConstraints.These cases depend on
Version,ExcludedVersions, andNoPrereleases; a package-level sentinel will make a future field rename fail at compile time instead of quietly weakening the test. As per coding guidelines,**/*_test.go: “when a test uses a specific struct field … add … a compile guard so a field rename immediately fails the build.”🤖 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/vendoring/version/version_test.go` around lines 81 - 138, Add a package-level compile-time sentinel in version_test.go to lock the test against schema.VendorConstraints field renames; the cases in TestResolveVersionConstraints rely on Version, ExcludedVersions, and NoPrereleases, so introduce a guard near the test setup that references those fields on schema.VendorConstraints and will fail to build if any of them change. Keep the existing ResolveVersionConstraints test logic unchanged, but ensure the sentinel is in the same test package so future field renames are caught at compile time.Source: Coding guidelines
🤖 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/vendor/diff.go`:
- Around line 66-71: The vendor diff command is registering its command-specific
flags directly on Cobra instead of using the shared standard parser. Update the
flag wiring in init() for vendorDiffCmd to route these options through
flags.NewStandardParser(), consistent with other cmd/** commands. Keep the
existing flag names and behavior for component, from, to, diff-file, and file,
but move their registration onto the standard parser path so config/flag
handling stays uniform.
In `@cmd/vendor/update.go`:
- Around line 60-72: The vendor update command is registering its own flags
directly on Cobra instead of using the standard parser path. Update the command
setup in init() for vendorUpdateCmd so command-specific flags are defined
through flags.NewStandardParser() like vendor diff, and route the existing
component/type/tags/check/pull/outdated/file/stack/everything/dry-run options
through that parser. Keep the same flag names and behavior, but remove the
direct Cobra flag registration from this command.
- Around line 36-46: The update flow is ignoring the requested type, so `--type`
currently has no effect during `vendoring.Update`. Update the
`cmd/vendor/update.go` path that builds `vendoring.UpdateParams` to either
remove the unused flag from the update command or pass it through and use it
inside `vendoring.Update`/related helpers so only the matching manifest entries
are rewritten. Also apply the same fix in the other update call site referenced
by the duplicate comment.
In `@pkg/vendoring/diff.go`:
- Around line 158-166: The path filtering in selectFileSections is too loose
because strings.Contains on the diff --git header can match unrelated files that
merely include the filter text. Update selectFileSections to parse the diff
--git a/... b/... header and compare the selected file path exactly against the
target passed to --diff-file, using the existing selectFileSections helper and
its keep logic to preserve only the intended hunks.
In `@pkg/vendoring/files_test.go`:
- Around line 61-70: The test for readVendorSources currently verifies
Component, Version, and Tags but misses Targets, allowing the string-targets
mapping in readVendorSources to regress unnoticed. Update
TestReadVendorSources_DecodesStringTargets to assert sources[0].Targets as well,
using the same importedManifest fixture, so the schema.AtmosVendorSource mapping
in files.go is covered and the string targets contract is enforced.
- Around line 54-58: The CollectManifestFiles tests are too loose because they
only check slice length and a permissive path predicate, which can hide ordering
or resolution regressions. Tighten the assertions in files_test.go around the
CollectManifestFiles call sites by verifying the exact first and last returned
entries by value, using the existing main/file variables and any known expected
terraform.yaml path, so wrong paths, duplicates, or ordering changes fail the
test.
In `@pkg/vendoring/update_test.go`:
- Around line 96-105: The dry-run test is ignoring possible failures from
os.ReadFile, which can make the before/after comparison pass incorrectly. In
TestUpdate_DryRunDoesNotWrite, add require.NoError checks for both fixture reads
around the existing before and after variables before asserting equality, so any
read failure fails the test immediately.
In `@pkg/vendoring/update.go`:
- Around line 27-35: The declaration comment on SourceUpdateResult.Reason needs
to satisfy godot by ending with a period and using proper capitalization. Update
the inline field comment on Reason to read like a complete sentence with an
initial capital letter and a trailing period, keeping it in the
SourceUpdateResult struct.
- Around line 111-126: The `Update` flow is swallowing hard failures by
converting `resolveLatest` and `SetComponentVersion` errors into
`StatusSkipped`, which makes callers think the run succeeded. Update `Update` to
return the report together with an error when these failures occur instead of
downgrading them, and keep `res.Reason` for reporting. Use the existing
`resolveLatest` and `SetComponentVersion` paths as the failure points, and wrap
all returned errors with the static errors from `errors/errors.go`, combining
any multiple failures with `errors.Join`.
In `@pkg/vendoring/version/remote.go`:
- Around line 56-76: `ExtractGitURI` is leaving Terraform subdirectory segments
in the normalized remote URL, so tag lookup still targets a module path instead
of the repo root. Update the normalization logic in `ExtractGitURI` to detect
and remove any `//subdir` suffix that appears before query parameters, while
preserving the repo URL, `git::` handling, `github.com/` shorthand conversion,
and `.git` canonicalization. Make sure the cleaned result for sources like
`github.com/org/repo//modules/vpc?ref=v1.2.3` points to the root repository so
the remote lister works correctly.
---
Nitpick comments:
In `@pkg/vendoring/version/version_test.go`:
- Around line 81-138: Add a package-level compile-time sentinel in
version_test.go to lock the test against schema.VendorConstraints field renames;
the cases in TestResolveVersionConstraints rely on Version, ExcludedVersions,
and NoPrereleases, so introduce a guard near the test setup that references
those fields on schema.VendorConstraints and will fail to build if any of them
change. Keep the existing ResolveVersionConstraints test logic unchanged, but
ensure the sentinel is in the same test package so future field renames are
caught at compile time.
🪄 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: e8589560-fb32-48a7-a15d-6861e4f751ee
📒 Files selected for processing (21)
cmd/vendor/diff.gocmd/vendor/update.gocmd/vendor/vendor.godocs/prd/vendor-update.mderrors/errors.gopkg/datafetcher/schema/vendor/package/1.0.jsonpkg/schema/schema.gopkg/vendoring/diff.gopkg/vendoring/diff_test.gopkg/vendoring/files.gopkg/vendoring/files_test.gopkg/vendoring/update.gopkg/vendoring/update_test.gopkg/vendoring/version/check.gopkg/vendoring/version/constraints.gopkg/vendoring/version/remote.gopkg/vendoring/version/version_test.gowebsite/blog/2026-06-27-yaml-editing-config-stack-vendor.mdxwebsite/docs/cli/commands/vendor/vendor-diff.mdxwebsite/docs/cli/commands/vendor/vendor-update.mdxwebsite/src/data/roadmap.js
✅ Files skipped from review due to trivial changes (3)
- website/docs/cli/commands/vendor/vendor-diff.mdx
- website/blog/2026-06-27-yaml-editing-config-stack-vendor.mdx
- website/src/data/roadmap.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@pkg/vendoring/edit_test.go`:
- Around line 96-106: The invalid YAML test currently only checks that
SetComponentVersion does not return atmosyaml.ErrYAMLPathNotFound, which is too
weak; update TestSetComponentVersion_InvalidYAML to also assert the returned
error matches the malformed-YAML parse failure contract. Use the
SetComponentVersion call and the existing atmosyaml error symbols to verify the
error is specifically a YAML parse error, not just any non-path-not-found
failure, so the test pins the actual cause explicitly.
In `@pkg/yaml/path.go`:
- Around line 168-178: QuotePathSegment currently wraps non-simple keys in
double quotes without escaping embedded quote or backslash characters, which can
break yq paths built by BuildComponentYqPath. Update QuotePathSegment to escape
any internal double quotes and backslashes before surrounding the key with
quotes, while keeping simple identifiers unchanged. Verify the escaping behavior
for component names that contain characters like foo"bar and foo\bar so the
rendered path stays valid and targets the intended key.
🪄 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: fa18d944-1cb5-40e5-b275-866f02fc896a
📒 Files selected for processing (19)
cmd/stack/operations.godemo/screengrabs/demo-stacks.txtpkg/config/config_edit.gopkg/config/config_edit_test.gopkg/stack/edit.gopkg/stack/edit_test.gopkg/vendoring/edit.gopkg/vendoring/edit_test.gopkg/yaml/edit.gopkg/yaml/edit_test.gopkg/yaml/path.gowebsite/docs/cli/commands/config/config-delete.mdxwebsite/docs/cli/commands/config/config-get.mdxwebsite/docs/cli/commands/config/config-set.mdxwebsite/docs/cli/commands/stack/stack-delete.mdxwebsite/docs/cli/commands/stack/stack-get.mdxwebsite/docs/cli/commands/stack/stack-set.mdxwebsite/docs/cli/commands/vendor/vendor-get.mdxwebsite/docs/cli/commands/vendor/vendor-set.mdx
✅ Files skipped from review due to trivial changes (8)
- website/docs/cli/commands/vendor/vendor-get.mdx
- website/docs/cli/commands/config/config-set.mdx
- website/docs/cli/commands/vendor/vendor-set.mdx
- website/docs/cli/commands/config/config-get.mdx
- website/docs/cli/commands/stack/stack-set.mdx
- website/docs/cli/commands/config/config-delete.mdx
- website/docs/cli/commands/stack/stack-get.mdx
- website/docs/cli/commands/stack/stack-delete.mdx
🚧 Files skipped from review as they are similar to previous changes (6)
- pkg/stack/edit.go
- pkg/config/config_edit_test.go
- pkg/stack/edit_test.go
- pkg/vendoring/edit.go
- pkg/config/config_edit.go
- cmd/stack/operations.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2664 +/- ##
==========================================
+ Coverage 81.26% 81.28% +0.02%
==========================================
Files 1563 1624 +61
Lines 149356 153050 +3694
==========================================
+ Hits 121371 124413 +3042
- Misses 21366 21811 +445
- Partials 6619 6826 +207
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/vendor/vendor_test.go`:
- Around line 136-176: The tests mutate shared package-level Cobra command
instances, so the flag state on vendorDiffCmd and vendorUpdateCmd leaks into
later tests and makes them order-dependent. Update these tests to use
cmd.NewTestKit(t) so RootCmd state is auto-cleaned, or otherwise snapshot and
restore every touched flag via t.Cleanup around the specific RunE calls on
vendorDiffCmd and vendorUpdateCmd.
- Around line 179-190: The current test only verifies that renderUpdateReport
runs without panicking, so it misses regressions in the rendered content. Update
TestRenderUpdateReport_AllStatuses to capture the writer output from
renderUpdateReport and assert the expected rows for each status path (updated,
current, skipped, failed), including component names and version/reason text,
using renderUpdateReport and vendoring.UpdateReport as the key entry points.
🪄 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: f0d3dc8f-613b-460a-9317-45f1d2a24ec6
📒 Files selected for processing (19)
cmd/config/operations_test.gocmd/stack/operations_test.gocmd/vendor/diff.gocmd/vendor/edit.gocmd/vendor/update.gocmd/vendor/vendor_test.goerrors/errors.gopkg/config/config_edit_test.gopkg/vendoring/diff.gopkg/vendoring/diff_test.gopkg/vendoring/edit_test.gopkg/vendoring/files.gopkg/vendoring/files_test.gopkg/vendoring/update.gopkg/vendoring/update_test.gopkg/vendoring/version/remote.gopkg/vendoring/version/version_test.gopkg/yaml/edit_test.gopkg/yaml/path.go
🚧 Files skipped from review as they are similar to previous changes (12)
- errors/errors.go
- cmd/vendor/update.go
- cmd/vendor/diff.go
- cmd/vendor/edit.go
- pkg/vendoring/edit_test.go
- pkg/vendoring/version/remote.go
- pkg/yaml/path.go
- pkg/vendoring/diff.go
- pkg/vendoring/update.go
- pkg/vendoring/version/version_test.go
- pkg/config/config_edit_test.go
- pkg/vendoring/files.go
The "Version Tracker E2E" job failed with "Unknown command build for atmos" because .atmos.d/build.yaml's working_directory used the new !repo-root YAML tag, which doesn't exist in the pinned CI bootstrap binary (ATMOS_BOOTSTRAP_VERSION: 1.222.0 -- confirmed via `git show v1.222.0:pkg/function/tag/tag.go`, no repo-root registration). Since that bootstrap binary is what builds atmos from source in CI, it couldn't parse the custom command using a tag newer than itself. Move repo-root resolution into the shell layer instead (git rev-parse --show-toplevel in scripts/build-atmos.sh and each subcommand's inline step), so the bootstrap-critical "build" command has no dependency on any atmos YAML feature newer than the pinned bootstrap version. Verified atmos build/deps/version all still resolve to the repo root correctly. Also fixes "Check Markdown Links": a link to a real, valid GitHub release page (v1.203.0, verified reachable via gh api and curl) hit a transient 500 from GitHub's gateway in CI. Added a scoped exclusion to lychee.toml matching the repo's existing pattern for known-flaky external URLs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous fix (5e53cb1) only removed !repo-root from .atmos.d/build.yaml, but .atmos.d/dev.yaml also used it (in "dev generate notices" and "dev generate snapshots"), which kept breaking the Version Tracker E2E job's "atmos build" bootstrap step with "Unknown command build for atmos". Root-caused by downloading the actual pinned bootstrap binary (v1.222.0) and bisecting .atmos.d/*.yaml against it: any file in .atmos.d/ containing an unsupported YAML tag breaks the *entire* custom-command merge, not just the offending file -- which is why "build" vanished from the command list even though build.yaml itself was already clean. "notices" already had its repo root resolved internally by scripts/generate-notice.sh, so working_directory was redundant there; removed it. "snapshots" needed the cd moved into its inline shell step via git rev-parse --show-toplevel, matching the pattern already used in build.yaml/build-atmos.sh. Verified by running the real v1.222.0 binary against the repo: `atmos build` now succeeds end-to-end. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
changed the title
Makes atmos vendor get|set <component> a literal thin wrapper over
atmos vendor config get|set <path>, matching how atmos stack
get|set|delete|format already alias atmos stack config
get|set|delete|format (same free functions under both entry points).
Previously vendor get/set were NOT aliases: they built a yq
select-by-component-name expression and evaluated it via
atmosyaml.QueryFile/EvalFile, while vendor config took a literal
dot-notation path and called atmosyaml.GetFile/SetFileWithType --
two different underlying primitives that happened to overlap for the
single-component-version use case.
- pkg/vendoring/files.go: add ComponentVersionPath(vendorFile,
component) resolving a component name to its spec.sources[N].version
dot-path (matched by name via readVendorSources, so manifest
ordering still doesn't matter -- the index is resolved fresh on
every call, never cached).
- cmd/vendor/config.go: extract runVendorConfigGet/runVendorConfigSet
free functions, mirroring runStackGet/runStackSet.
- cmd/vendor/edit.go: vendorGetCmd/vendorSetCmd now resolve the
component's path via ComponentVersionPath and delegate to the same
runVendorConfigGet/runVendorConfigSet the config commands use.
- pkg/vendoring/edit.go deleted (GetComponentVersion,
selectByComponent, yqStringLiteral -- confirmed dead after the
refactor). SetComponentVersion had one other caller
(pkg/vendoring/update.go) and was kept, reimplemented on top of the
same ComponentVersionPath + atmosyaml.SetFileWithType rather than
the old yq-expression path, so vendor update's internal
version-writes also now go through the canonical path-based engine.
- New test proving the alias property end-to-end (same component,
same manifest, vendor get/set and vendor config get/set produce
identical results); ported SetComponentVersion's existing test
cases against the new implementation.
Also documents the previously-undocumented atmos stack config * and
atmos vendor config * command groups (added to the branch after the
original PRD/blog/CLI docs were written, never given Docusaurus
pages), and reframes config get|set|delete|format|list as the
canonical command set for config/stack/vendor, with the non-config
shorthand commands documented as aliases:
- New website/docs/cli/commands/{stack,vendor}/config/ subdirectories
(get/set/delete/format/list + overview), following the existing
terraform/cache/ subdirectory precedent.
- New pages for previously-undocumented commands: atmos stack format,
atmos config list, atmos config format.
- Alias cross-link notes added to the 5 existing flat command pages.
- docs/prd/yaml-editing-get-set.md updated to describe the full
current surface.
Verified: go build/test clean, atmos lint changed clean, manual
demonstration that vendor get/set and vendor config get/set agree on
the same component without disturbing neighboring sources, and
`cd website && npm run build` passes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
cmd/markdown/atmos_stack_format_usage.md (1)
3-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the command examples valid Markdown code blocks.
Use a
bashfence and drop the$prompts so the snippets render cleanly and stay copy/pasteable.Suggested fix
-``` - $ atmos stack format -s plat-ue2-prod -c vpc -``` +```bash +atmos stack format -s plat-ue2-prod -c vpc +``` @@ -``` - $ atmos stack format -s plat-ue2-prod -c vpc --file stacks/catalog/vpc.yaml -``` +```bash +atmos stack format -s plat-ue2-prod -c vpc --file stacks/catalog/vpc.yaml +``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>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/markdown/atmos_stack_format_usage.mdaround lines 3 - 10, The command
examples in atmos_stack_format_usage.md are not valid Markdown code blocks;
update the two atmos stack format examples to use bash-fenced code blocks and
remove the leading $ prompts so they render cleanly and remain copy/pasteable.</details> <!-- cr-comment:v1:c64f81537a11aba41120f787 --> </blockquote></details> <details> <summary>cmd/markdown/atmos_stack_config_format_usage.md (1)</summary><blockquote> `3-10`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **Make the command examples valid Markdown code blocks.** Use a `bash` fence and drop the `$` prompts so the snippets render cleanly and stay copy/pasteable. <details> <summary>Suggested fix</summary> ```diff -``` - $ atmos stack config format -s plat-ue2-prod -c vpc -``` +```bash +atmos stack config format -s plat-ue2-prod -c vpc +``` @@ -``` - $ atmos stack config format -s plat-ue2-prod -c vpc --file stacks/catalog/vpc.yaml -``` +```bash +atmos stack config format -s plat-ue2-prod -c vpc --file stacks/catalog/vpc.yaml +``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>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/markdown/atmos_stack_config_format_usage.mdaround lines 3 - 10, The
command examples in the markdown doc are not valid fenced code blocks, so update
the examples under atmos_stack_config_format_usage to use bash fences and remove
the shell prompt characters. Make both command snippets render as standalone
copy/pasteable code blocks by adjusting the markdown around the atmos stack
config format examples.</details> <!-- cr-comment:v1:0184fb08e2053e05ac4ca9ba --> </blockquote></details> <details> <summary>cmd/markdown/atmos_config_format_usage.md (1)</summary><blockquote> `3-10`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **Make the command examples valid Markdown code blocks.** Use a `bash` fence and drop the `$` prompts so the snippets render cleanly and stay copy/pasteable. <details> <summary>Suggested fix</summary> ```diff -``` - $ atmos config format -``` +```bash +atmos config format +``` @@ -``` - $ atmos --config ./config/atmos.yaml config format -``` +```bash +atmos --config ./config/atmos.yaml config format +``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>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/markdown/atmos_config_format_usage.mdaround lines 3 - 10, The command
examples in the markdown usage doc are not valid fenced code blocks, so update
the examples in atmos_config_format_usage.md to use proper bash fences and
remove the shell prompt markers. Fix the two snippet blocks around the atmos
config format and atmos --config ./config/atmos.yaml config format examples so
they render as clean, copy/pasteable Markdown.</details> <!-- cr-comment:v1:ac99bfb82e20a384d9cfdea8 --> </blockquote></details> <details> <summary>cmd/markdown/atmos_vendor_config_format_usage.md (1)</summary><blockquote> `3-10`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **Make the command examples valid Markdown code blocks.** Use a `bash` fence and drop the `$` prompts so the snippets render cleanly and stay copy/pasteable. <details> <summary>Suggested fix</summary> ```diff -``` - $ atmos vendor config format -``` +```bash +atmos vendor config format +``` @@ -``` - $ atmos vendor config format --file ./vendor.yaml -``` +```bash +atmos vendor config format --file ./vendor.yaml +``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>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/markdown/atmos_vendor_config_format_usage.mdaround lines 3 - 10, The
command examples in the markdown are not valid fenced code blocks, which hurts
rendering and copy/paste. Update the examples in the vendor config format docs
to use proper bash-fenced code blocks and remove the shell prompt characters,
keeping the snippets clean and consistent foratmos vendor config formatand
atmos vendor config format --file ./vendor.yaml.</details> <!-- cr-comment:v1:9a11fe16e91cd246847ede64 --> </blockquote></details> <details> <summary>cmd/stack/config.go (2)</summary><blockquote> `96-101`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **Consolidate duplicated list/path-formatting helpers.** `stackPathPatternArg`, the entry→`PathRow` building loop, and `relativePathForStackDisplay` are near-duplicates of `pathPatternArg`, `buildConfigPathRows`'s loop, and `relativePathForDisplay` in `cmd/config/list.go` (the guard-clause ordering already diverged slightly between the two `relativePath*` variants). The `componentType` extraction here also mirrors the same snippet in `cmd/stack/operations.go` (Line 184). Given the PR also adds an analogous `vendor config list`, consider hoisting these into `pkg/list` (or a shared internal helper) to avoid a third copy and future divergence. Also applies to: 133-193, 195-204 <details> <summary>🤖 Prompt for AI Agents</summary>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/stack/config.goaround lines 96 - 101,stackPathPatternArg, the
PathRowconstruction loop, andrelativePathForStackDisplayare duplicating
logic already present incmd/config/list.goandcmd/stack/operations.go.
Refactor these shared path/list helpers into a common place such aspkg/list
or another internal helper, then have the stack commands call the shared
functions instead of maintaining their own copies. Keep thecomponentType
extraction and relative-path formatting behavior consistent across
stackPathPatternArg,buildConfigPathRows, andrelativePathForDisplayto
prevent future divergence.</details> <!-- cr-comment:v1:440af50eed9e385d9116a3e2 --> --- `76-95`: _🎯 Functional Correctness_ | _🔵 Trivial_ | _⚡ Quick win_ **`stack config list`'s `--format`/`--delimiter` bypass Viper, unlike `atmos config list`.** `cmd/config/list.go` binds its equivalent flags through `flags.StandardParser` + Viper so they can be overridden via environment variables. Here they're plain `StringVarP` bindings with no Viper wiring, so env-var overrides silently won't work for this subcommand. As per coding guidelines: "Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults." Worth confirming this asymmetry between the two nearly-identical `list` commands is intentional. Also applies to: 110-113 <details> <summary>🤖 Prompt for AI Agents</summary>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/stack/config.goaround lines 76 - 95, ThestackConfigListCmdflag
setup for--formatand--delimiteris bypassing Viper, so
environment-variable overrides won’t work like they do inconfigListCmd.
Update thestack config listcommand wiring to use the same
flags.StandardParser/Viper binding approach ascmd/config/list.go, and
ensureRunEstill reads the resolved values through the existingflagFormat
andflagDelimitervariables.</details> <!-- cr-comment:v1:34d6272589ec05db2512ce8a --> _Source: Coding guidelines_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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/config/list_test.go:
- Around line 69-90: The test setup in configListCmd.RunE initializes the global
data writer via data.InitWriter but never resets it, so add
t.Cleanup(data.Reset) right after initialization to avoid leaking writer state
into other tests. Use the existing configListCmd.RunE test as the fix location
and follow the same cleanup pattern used by the analogous test helper in other
cmd/* tests.
Nitpick comments:
In@cmd/markdown/atmos_config_format_usage.md:
- Around line 3-10: The command examples in the markdown usage doc are not valid
fenced code blocks, so update the examples in atmos_config_format_usage.md to
use proper bash fences and remove the shell prompt markers. Fix the two snippet
blocks around the atmos config format and atmos --config ./config/atmos.yaml
config format examples so they render as clean, copy/pasteable Markdown.In
@cmd/markdown/atmos_stack_config_format_usage.md:
- Around line 3-10: The command examples in the markdown doc are not valid
fenced code blocks, so update the examples under atmos_stack_config_format_usage
to use bash fences and remove the shell prompt characters. Make both command
snippets render as standalone copy/pasteable code blocks by adjusting the
markdown around the atmos stack config format examples.In
@cmd/markdown/atmos_stack_format_usage.md:
- Around line 3-10: The command examples in atmos_stack_format_usage.md are not
valid Markdown code blocks; update the two atmos stack format examples to use
bash-fenced code blocks and remove the leading $ prompts so they render cleanly
and remain copy/pasteable.In
@cmd/markdown/atmos_vendor_config_format_usage.md:
- Around line 3-10: The command examples in the markdown are not valid fenced
code blocks, which hurts rendering and copy/paste. Update the examples in the
vendor config format docs to use proper bash-fenced code blocks and remove the
shell prompt characters, keeping the snippets clean and consistent foratmos vendor config formatandatmos vendor config format --file ./vendor.yaml.In
@cmd/stack/config.go:
- Around line 96-101:
stackPathPatternArg, thePathRowconstruction loop,
andrelativePathForStackDisplayare duplicating logic already present in
cmd/config/list.goandcmd/stack/operations.go. Refactor these shared
path/list helpers into a common place such aspkg/listor another internal
helper, then have the stack commands call the shared functions instead of
maintaining their own copies. Keep thecomponentTypeextraction and
relative-path formatting behavior consistent acrossstackPathPatternArg,
buildConfigPathRows, andrelativePathForDisplayto prevent future
divergence.- Around line 76-95: The
stackConfigListCmdflag setup for--formatand
--delimiteris bypassing Viper, so environment-variable overrides won’t work
like they do inconfigListCmd. Update thestack config listcommand wiring
to use the sameflags.StandardParser/Viper binding approach as
cmd/config/list.go, and ensureRunEstill reads the resolved values through
the existingflagFormatandflagDelimitervariables.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `ee81a9bf-ae11-48f5-9365-d50bd79d800e` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c1922d62fb582b3ed7f00520858ec34c8d285a60 and f02e4e82bd463bb418932dd23b16b09d72512fa1. </details> <details> <summary>📒 Files selected for processing (44)</summary> * `.atmos.d/build.yaml` * `.atmos.d/dev.yaml` * `.github/workflows/dependency-review.yml` * `.github/workflows/setup-go-cache-warmup.yml` * `.github/workflows/version-tracker.yaml` * `.gitignore` * `.golangci.yml` * `NOTICE` * `cmd/config/config.go` * `cmd/config/list.go` * `cmd/config/list_test.go` * `cmd/config/operations.go` * `cmd/config/operations_test.go` * `cmd/markdown/atmos_config_format_usage.md` * `cmd/markdown/atmos_stack_config_format_usage.md` * `cmd/markdown/atmos_stack_format_usage.md` * `cmd/markdown/atmos_vendor_config_format_usage.md` * `cmd/root.go` * `cmd/stack/config.go` * `cmd/stack/config_test.go` * `cmd/stack/operations.go` * `cmd/stack/operations_test.go` * `cmd/stack/stack.go` * `cmd/vendor/config.go` * `cmd/vendor/config_test.go` * `cmd/vendor/edit.go` * `cmd/vendor/vendor_test.go` * `cmd/version/track/add.go` * `cmd/version/track/apply.go` * `cmd/version/track/diff.go` * `cmd/version/track/get.go` * `cmd/version/track/list.go` * `cmd/version/track/lock.go` * `cmd/version/track/remove.go` * `cmd/version/track/render.go` * `cmd/version/track/set.go` * `cmd/version/track/show.go` * `cmd/version/track/status.go` * `cmd/version/track/track.go` * `cmd/version/track/track_test.go` * `cmd/version/track/update.go` * `cmd/version/track/verify.go` * `cmd/version/version.go` * `demo/screengrabs/demo-stacks.txt` </details> <details> <summary>💤 Files with no reviewable changes (23)</summary> * cmd/version/track/set.go * cmd/version/version.go * cmd/version/track/remove.go * cmd/version/track/verify.go * cmd/version/track/show.go * demo/screengrabs/demo-stacks.txt * cmd/version/track/lock.go * cmd/version/track/diff.go * cmd/version/track/list.go * cmd/version/track/update.go * cmd/version/track/add.go * cmd/version/track/apply.go * cmd/stack/stack.go * cmd/version/track/track.go * cmd/version/track/status.go * cmd/version/track/get.go * cmd/version/track/render.go * cmd/vendor/edit.go * cmd/version/track/track_test.go * cmd/vendor/config_test.go * cmd/vendor/config.go * cmd/vendor/vendor_test.go * cmd/stack/operations_test.go </details> <details> <summary>✅ Files skipped from review due to trivial changes (2)</summary> * .gitignore * NOTICE </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (3)</summary> * cmd/config/config.go * cmd/config/operations.go * cmd/root.go </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+69
to
+90
| func TestConfigListCommand_RunE(t *testing.T) { | ||
| // configListCmd.RunE ends up calling data.Write, which panics unless the | ||
| // I/O writer has been initialized (mirrors the pattern used across other | ||
| // cmd/* packages, e.g. cmd/ai/skill/list_test.go, cmd/list/components_test.go). | ||
| ioCtx, err := iolib.NewContext() | ||
| require.NoError(t, err) | ||
| data.InitWriter(ioCtx) | ||
|
|
||
| dir := t.TempDir() | ||
| file := filepath.Join(dir, "atmos.yaml") | ||
| require.NoError(t, os.WriteFile(file, []byte("logs:\n level: info\n"), 0o644)) | ||
|
|
||
| wd, err := os.Getwd() | ||
| require.NoError(t, err) | ||
| t.Cleanup(func() { | ||
| require.NoError(t, os.Chdir(wd)) | ||
| }) | ||
| require.NoError(t, os.Chdir(dir)) | ||
|
|
||
| require.NoError(t, configListCmd.RunE(configListCmd, nil)) | ||
| require.NoError(t, configListCmd.RunE(configListCmd, []string{"logs.*"})) | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Missing t.Cleanup(data.Reset) after data.InitWriter.
The global writer set here isn't reset, unlike the analogous helper in cmd/stack/config_test.go (initStackConfigTestWriter), which explicitly calls t.Cleanup(data.Reset). Without cleanup, this global state can leak into other tests in the same package/binary.
🧹 Proposed fix
ioCtx, err := iolib.NewContext()
require.NoError(t, err)
data.InitWriter(ioCtx)
+ t.Cleanup(data.Reset)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func TestConfigListCommand_RunE(t *testing.T) { | |
| // configListCmd.RunE ends up calling data.Write, which panics unless the | |
| // I/O writer has been initialized (mirrors the pattern used across other | |
| // cmd/* packages, e.g. cmd/ai/skill/list_test.go, cmd/list/components_test.go). | |
| ioCtx, err := iolib.NewContext() | |
| require.NoError(t, err) | |
| data.InitWriter(ioCtx) | |
| dir := t.TempDir() | |
| file := filepath.Join(dir, "atmos.yaml") | |
| require.NoError(t, os.WriteFile(file, []byte("logs:\n level: info\n"), 0o644)) | |
| wd, err := os.Getwd() | |
| require.NoError(t, err) | |
| t.Cleanup(func() { | |
| require.NoError(t, os.Chdir(wd)) | |
| }) | |
| require.NoError(t, os.Chdir(dir)) | |
| require.NoError(t, configListCmd.RunE(configListCmd, nil)) | |
| require.NoError(t, configListCmd.RunE(configListCmd, []string{"logs.*"})) | |
| } | |
| func TestConfigListCommand_RunE(t *testing.T) { | |
| // configListCmd.RunE ends up calling data.Write, which panics unless the | |
| // I/O writer has been initialized (mirrors the pattern used across other | |
| // cmd/* packages, e.g. cmd/ai/skill/list_test.go, cmd/list/components_test.go). | |
| ioCtx, err := iolib.NewContext() | |
| require.NoError(t, err) | |
| data.InitWriter(ioCtx) | |
| t.Cleanup(data.Reset) | |
| dir := t.TempDir() | |
| file := filepath.Join(dir, "atmos.yaml") | |
| require.NoError(t, os.WriteFile(file, []byte("logs:\n level: info\n"), 0o644)) | |
| wd, err := os.Getwd() | |
| require.NoError(t, err) | |
| t.Cleanup(func() { | |
| require.NoError(t, os.Chdir(wd)) | |
| }) | |
| require.NoError(t, os.Chdir(dir)) | |
| require.NoError(t, configListCmd.RunE(configListCmd, nil)) | |
| require.NoError(t, configListCmd.RunE(configListCmd, []string{"logs.*"})) | |
| } |
🤖 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/config/list_test.go` around lines 69 - 90, The test setup in
configListCmd.RunE initializes the global data writer via data.InitWriter but
never resets it, so add t.Cleanup(data.Reset) right after initialization to
avoid leaking writer state into other tests. Use the existing configListCmd.RunE
test as the fix location and follow the same cleanup pattern used by the
analogous test helper in other cmd/* tests.
Andriy Knysh (aknysh)
approved these changes
Jul 8, 2026
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
Warning Release Documentation RequiredThis PR is labeled
|
|
These changes were released in v1.223.0-rc.6. |
This was referenced Jul 9, 2026
This was referenced Aug 5, 2026
Andriy Knysh (aknysh)
added a commit
to zack-is-cool/atmos
that referenced
this pull request
Aug 11, 2026
…lution (cloudposse#2900) * fix(version): exclude draft GitHub releases from Version Tracker resolution github-releases datasource resolution could pick an unpublished draft release instead of the latest published one when the GitHub token had repo write access (e.g. secrets.GITHUB_TOKEN in CI). GetReleases now filters out drafts unconditionally, alongside the existing prerelease filter, fixing the Version Tracker resolver, `atmos version list`, and GetReleaseVersions at the shared root cause. * fix(version): remove the deprecated `atmos version track render` command render.go and apply.go were added in the same commit that introduced the Version Tracker (cloudposse#2664); render was marked Deprecated/Hidden from day one and has never had a non-deprecated existence in any release, so it never needs a migration path. Relocates the shared renderTemplate helper into apply.go (its only remaining consumer) and removes the now-dead manager.RenderFile helper and render-specific sentinel errors. Also fixes pre-existing EditorConfig indentation drift in docs/prd/atmos-version-management.md (3-space list/fence indents under numbered items instead of the required 2-space multiple), surfaced by this branch's `--affected` validation once the file was touched. * docs(version): expand version.files and !version function examples Adds worked before/after examples for the marker and github-actions file managers (including SHA pinning) to the version.files reference, and adds Helm/Container-component tabs to the !version function docs alongside the existing Terraform example, so each supported component type has a concrete resolution example rather than relying solely on the Terraform case. * test(github): assert filterDrafts output tags and order, not just length Length-only assertions let filterDrafts silently drop or reorder the wrong releases; assert each result's tag against the expected order. * docs(version): refine file-manager and !version examples - Replace before/after tabs in files.mdx with one tab per distinct scenario (single tool, multiple tools, YAML, custom match pattern, version bump, pinned to SHA), and show both trailing and standalone marker comment forms for the match= example. - Use current tool versions in examples instead of stale ones. - Rename the "Non-Dockerfile Format" tab to "YAML" to name what the example is instead of what it isn't. - Clarify in version.mdx that !version only resolves inside stack manifests, not in atmos.yaml or arbitrary YAML files. * docs(version): address CodeRabbit review findings on PR cloudposse#2900 - Qualify the GITHUB_TOKEN permission claim in the draft-exclusion fix log: repo CI only sees drafts when its token permissions grant read access to contents, not unconditionally. - Fix the pinned-SHA example in files.mdx: the SHA labeled v6.1.0 was actually actions/checkout's v5.0.0 release commit. Swapped in the correct SHA for the v6.1.0 tag. * fix(security): remediate 7 Dependabot alerts in website deps Bump pnpm overrides to patched versions, all within their existing major line (not blocked by dependabot.yml's semver-major ignore): - js-yaml 3.15.0 -> 3.15.1 (GHSA-5p4m-2wfm-xmqj, high) - js-yaml 4.3.0 -> 4.3.1 (high) - mermaid 11.16.0 -> 11.16.1 (4 advisories: 1 low, 3 moderate) * fix(ci): isolate merge_group base-resolution test from ambient GITHUB_EVENT_PATH TestSetDescribeAffectedFlagValueInCliArgs_BaseResolution's merge_group subtest only stubbed GITHUB_EVENT_NAME/GITHUB_BASE_REF, leaving the real runner's GITHUB_EVENT_PATH in place. Under an actual merge_group-triggered job (i.e. the merge queue) that path points to a real payload carrying merge_group.base_sha, so resolution correctly takes the SHA branch and leaves describe.Ref empty -- failing the test's Ref assertion even though production behavior is correct. This is why the PR passed as a normal PR check but failed once queued. Clear GITHUB_EVENT_PATH explicitly, matching the pattern already used by sibling tests in this file and in pkg/ci/providers/github and pkg/validation for the same reason. * fix(security): remediate 5 CodeQL/Dependabot alerts Bump to patched versions, all within their existing major line (not blocked by dependabot.yml's semver-major ignore): - github.com/go-git/go-git/v5 v5.19.1 -> v5.19.2 (cloudposse#270 high, cloudposse#271 moderate), pulled transitively via `go get` + `go mod tidy`; regenerated NOTICE - dompurify 3.4.12 -> 3.4.13 (cloudposse#272 moderate) via pnpm override - nanoid 3.3.15 -> 3.3.18 (cloudposse#273, cloudposse#274 both high) via pnpm override; added a second override key matching postcss's own `^3.3.16` dependency range, which the existing `nanoid@3.3.3` key didn't catch image-size (cloudposse#275, cloudposse#276, both high) has no patched version published upstream yet -- not fixable. * fix(ci): widen Windows Acceptance tests timeout, root-caused via runner log Windows acceptance tests timed out at exactly the 60m step budget (started 19:04:21, `go test` itself finished 20:03:56 with every package printing `ok`, endgroup at 20:04:21, force-cancelled 17s later at 20:04:38). No test failure -- just CI-to-CI variance on a budget with no headroom, the same flake pattern already documented here from an earlier 45m->60m bump. Give windows-latest its own 80m step budget (matrix-conditional expression; macOS stays at 60m, where it comfortably finishes in ~35m today) and raise the job-level ceiling from 120m to 140m to preserve the same headroom-above-step-budget-sum margin the existing comment establishes for Linux and macOS. --------- Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.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
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
Format-preserving YAML editing — a shared, format-preserving YAML editing engine in
pkg/yaml(built on theyqliblibrary Atmos already vendors, fed raw bytes) withGet/Set/Delete/Eval/Query/Formatplus atomic file wrappers:!terraform.output,!env,!store, …), and Go/Gomplate templates ({{ … }}); a strict guard rejects edits that would alter or expand a YAML anchor.get|set|delete|format|listinterface and shorthand aliases for the common case:atmos config get|set|delete|format|list <path>— edits the activeatmos.yaml. (No separateconfigsub-namespace — this domain's flat form is the canonical form.)atmos stack config get|set|delete|format|list <path> -s <stack> -c <component>(canonical) /atmos stack get|set|delete|format(alias) — uses provenance to resolve the manifest that actually defines the effective (post-merge) value.listonly exists underconfig: a flatatmos stack listwould collide with the existingatmos list stacks.atmos vendor config get|set|delete|format|list <path>(canonical) /atmos vendor get|set <component> [version](alias) — the alias resolves the component name to itsspec.sources[N].versionpath and delegates to the same engine; the canonical form addresses any path in the manifest.atmos vendor update/atmos vendor diffon the same engine (check Git sources for newer versions, honoring semver constraints; diff two versions without a local checkout).--typeflag coerces values (string/int/bool/float/null/yaml).docs/prd/yaml-editing-get-set.md), Docusaurus docs, changelog blog post, and roadmap milestone.Atmos Version Tracker (#2688) — one catalog of software versions (local tools, GitHub Actions, OCI images, release tags, and other packages), declared under named tracks in
atmos.yaml, resolved into a deterministicversions.lock.yaml:atmos version track add|set|get|list|show|status|update|verify|remove|apply|rendercommands.@<sha> # <version>references.atmos version track apply --checkacts as a CI drift gate.docs/prd/atmos-version-management.md), Docusaurus docs, changelog blog post, and roadmap milestone.Supporting work merged onto this branch:
cmd/stack(46.8%→85.7%),cmd/config(61.7%→87.2%),cmd/vendor(62.1%→86.3%),pkg/yaml(94.4%→96.0%), andpkg/list/renderer(88.7%→92.7%) to close the gaps Codecov flagged on this PR's patch coverage.TestRelativePathForStackDisplayused synthetic paths that aren't recognized as absolute by Go's Windowsfilepath.IsAbs).go/allocation-size-overflowfinding ininternal/exec/utils.goand dismissed the associated alert..atmos.d/build.yamland.atmos.d/dev.yamlused the new!repo-rootYAML tag, which doesn't exist in the pinned olderatmosbootstrap binary CI uses to self-build from source — any occurrence anywhere in.atmos.d/was silently dropping the whole custom-command set. Moved repo-root resolution into the shell layer (git rev-parse --show-toplevel) instead.atmos vendor get|setrefactored from a separate yq-expression implementation into a literal thin wrapper overatmos vendor config get|set(matching howatmos stack get|set|delete|formatalready aliasatmos stack config), and documented the previously-undocumentedatmos stack config */atmos vendor config *command groups (13 new Docusaurus pages) that had been added to the branch after the original docs were written.why
sed/yqstrips comments and reformats files; a naive "parse → re-serialize" approach destroys comments, anchors, functions, and templates that carry real meaning and behavior. These commands let users and automation script configuration changes safely, at the YAML-node level, without losing fidelity. The sharedpkg/yamlengine generalizes (and supersedes) the hardcodedsources[].versionwriter from the olderfeat/vendor-diff-and-updatebranch.main) and merged in directly, so its full history and diff are part of this PR.references
docs/prd/yaml-editing-get-set.mddocs/prd/atmos-version-management.mdfeat/vendor-diff-and-updatebranch (the broadervendor diff/vendor updatefeature port is tracked as follow-up).gopkg.in/yaml.v3node model thatyqlibbuilds on).