Add vendor update and diff commands #1694
Conversation
what - Implement `atmos vendor update` command to check for and update vendored component versions - Implement `atmos vendor diff` command to show Git diffs between component versions - Add comprehensive test coverage (~80-85%) using gomock for mocking Git operations - Add complete documentation for both commands in Docusaurus format why - Users need to see what changed between vendored component versions before updating - Users need an easy way to update vendor.yaml version references - YAML structure, comments, and anchors must be preserved during updates - Git operations should work without requiring local repository clones references - Closes vendor update/diff feature request - See docs/prd/vendor-update.md for detailed design 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
📝 WalkthroughWalkthroughMoves and rewrites vendoring functionality: removes legacy internal/exec vendor CLI bits, adds a new pkg/vendoring package with Pull/Update/Diff entry points, provider-based Git/GitHub diffing, semver constraint resolution, YAML-preserving updates, exported exec helpers, tests, docs, and many vendor-related sentinel errors. Changes
Sequence Diagram(s)mermaid CLI->>VendorPkg: invoke vendor diff/pull/update (flags) Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas needing focused review:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (4){go.mod,go.sum}📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Files:
**/*.go📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Files:
pkg/**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
cmd/**/*.go📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Files:
🧠 Learnings (31)📓 Common learnings📚 Learning: 2025-11-24T17:35:37.209ZApplied to files:
📚 Learning: 2025-07-05T20:59:02.914ZApplied to files:
📚 Learning: 2025-01-17T00:21:32.987ZApplied to files:
📚 Learning: 2025-01-17T00:21:32.987ZApplied to files:
📚 Learning: 2025-12-06T19:24:49.329ZApplied to files:
📚 Learning: 2024-10-31T01:22:09.586ZApplied to files:
📚 Learning: 2025-12-06T19:24:49.329ZApplied to files:
📚 Learning: 2025-09-29T02:20:11.636ZApplied to files:
📚 Learning: 2025-12-06T19:24:49.329ZApplied to files:
📚 Learning: 2024-10-22T23:00:20.627ZApplied to files:
📚 Learning: 2024-12-02T21:26:32.337ZApplied to files:
📚 Learning: 2024-10-23T21:36:40.262ZApplied to files:
📚 Learning: 2025-04-04T02:03:23.676ZApplied to files:
📚 Learning: 2025-01-25T03:51:57.689ZApplied to files:
📚 Learning: 2025-01-17T00:18:57.769ZApplied to files:
📚 Learning: 2025-04-11T22:06:46.999ZApplied to files:
📚 Learning: 2025-03-18T12:26:25.329ZApplied to files:
📚 Learning: 2024-11-22T12:38:33.132ZApplied to files:
📚 Learning: 2025-10-10T23:51:36.597ZApplied to files:
📚 Learning: 2025-09-10T21:17:55.273ZApplied to files:
📚 Learning: 2024-10-12T18:38:28.458ZApplied to files:
📚 Learning: 2025-10-22T14:55:44.014ZApplied to files:
📚 Learning: 2025-11-24T17:35:37.209ZApplied to files:
📚 Learning: 2025-12-06T19:24:49.329ZApplied to files:
📚 Learning: 2024-12-11T18:40:12.808ZApplied to files:
📚 Learning: 2025-12-06T19:24:49.329ZApplied to files:
📚 Learning: 2025-01-30T19:30:59.120ZApplied to files:
📚 Learning: 2025-12-06T19:24:49.329ZApplied to files:
📚 Learning: 2025-09-13T16:39:20.007ZApplied to files:
📚 Learning: 2025-09-27T20:50:20.564ZApplied to files:
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go.mod (1)
3-3: Invalid Go version ingodirective (must be major.minor).
go 1.24.8will fail module parsing. Usego 1.24and, if you need to pin a patch, add atoolchaindirective.Apply:
-go 1.24.8 +go 1.24 +toolchain go1.24.8
🧹 Nitpick comments (12)
internal/exec/vendor_update_test.go (2)
56-95: Add parallelization and a couple of extra edge cases.
- Call
t.Parallel()at test start; helpers are pure.- Add cases like trailing separator (
"a,b,"), multiple separators ("a,,b"), and Unicode whitespace.Apply:
func TestSplitString(t *testing.T) { + t.Parallel()And extend the table with:
{ name: "trailing sep", input: "a,b,", sep: ",", expected: []string{"a","b",""} }, { name: "double sep", input: "a,,b", sep: ",", expected: []string{"a","","b"} },
97-141: Parallelize trim tests.Safe to run in parallel.
func TestTrimString(t *testing.T) { + t.Parallel()internal/exec/vendor_yaml_updater_test.go (2)
14-120: Good coverage; add explicit assertion that comments are preserved.Your “comments preserved” case only checks the version. Assert the comment still exists after update.
Apply:
@@ func TestUpdateYAMLVersion(t *testing.T) { - require.NoError(t, err) - - // Read back and verify version was updated + require.NoError(t, err) + + // Read back and verify version was updated. version, err := findComponentVersion(atmosConfig, tempFile, tt.componentName) require.NoError(t, err) assert.Equal(t, tt.newVersion, version) + + // If this case is about preserving comments, verify they remain. + if tt.name == "update version with comments preserved" { + b, rerr := os.ReadFile(tempFile) + require.NoError(t, rerr) + assert.Contains(t, string(b), "# current stable version") + }Also add
t.Parallel()at the top of the test.func TestUpdateYAMLVersion(t *testing.T) { + t.Parallel()
122-205: Consider adding tests for YAML anchors/aliases and directory BasePath.
- Add a case with anchors/aliases to ensure node updates preserve them.
- Add a case where
Vendor.BasePathpoints to a directory with multiple vendor files.Happy to draft these tests if you want.
website/docs/cli/commands/vendor/diff.mdx (1)
183-191: Minor style nit: ellipsis.If you care about typography, consider replacing
...with…in examples.internal/exec/vendor_diff_integration_test.go (1)
15-23: Run tests in parallel where safe.Top-level tests and subtests can parallelize; each uses TempDir and isolated mocks.
func TestExecuteVendorDiffWithGitOps(t *testing.T) { + t.Parallel() @@ - t.Run(tt.name, func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { + t.Parallel()func TestExecuteVendorDiffWithGitOps_DefaultFromVersion(t *testing.T) { + t.Parallel()Also applies to: 187-230
website/docs/cli/commands/vendor/update.mdx (1)
116-116: Use typographical ellipsis for better readability.Consider using the
…character instead of...for a more polished appearance.-- ✅ Git URLs (`git::https://...`, `git@...`) +- ✅ Git URLs (`git::https://…`, `git@…`)docs/prd/vendor-update.md (1)
60-79: Add fenced code languages and tidy punctuation.Add languages to fenced blocks (bash, diff, yaml, go, markdown). Also address a few missing sentence-ending periods and straight-quote nits flagged by linters.
Also applies to: 336-367, 386-405, 482-501, 513-541, 571-599, 649-668, 679-700, 712-726, 788-818, 975-1015
internal/exec/vendor_diff.go (2)
181-185: Wrap remote-tag lookup errors with sentinel.Use ErrGitLsRemoteFailed for consistency with centralized error handling.
Apply this diff:
- tags, err := gitOps.GetRemoteTags(gitURI) - if err != nil { - return fmt.Errorf("failed to get remote tags: %w", err) - } + tags, err := gitOps.GetRemoteTags(gitURI) + if err != nil { + return fmt.Errorf("%w: %s (%s)", errUtils.ErrGitLsRemoteFailed, err, gitURI) + }
200-214: Propagate diff errors with sentinel and include refs.Wrap with ErrGitDiffFailed to align with guidelines.
Apply this diff:
- diff, err := gitOps.GetDiffBetweenRefs(atmosConfig, gitURI, fromRef, toRef, flags.Context, flags.NoColor) - if err != nil { - return err - } + diff, err := gitOps.GetDiffBetweenRefs(atmosConfig, gitURI, fromRef, toRef, flags.Context, flags.NoColor) + if err != nil { + return fmt.Errorf("%w: %s..%s (%s): %s", errUtils.ErrGitDiffFailed, fromRef, toRef, gitURI, err) + }internal/exec/vendor_git_diff.go (2)
139-148: Use the central color decision helper.Reuse shouldColorizeOutput to keep logic consistent and honor TERM/TTY/no-color.
Apply this diff:
- // Add color if appropriate - if !noColor && isatty.IsTerminal(os.Stdout.Fd()) { - args = append(args, "--color=always") - } else { - args = append(args, "--color=never") - } + // Add color if appropriate. + if shouldColorizeOutput(noColor, "") { + args = append(args, "--color=always") + } else { + args = append(args, "--color=never") + }
168-177: ANSI stripper is too naive and unused.If needed, use a library (e.g., acarl005/stripansi) or remove until required.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
cmd/vendor_diff.go(2 hunks)cmd/vendor_update.go(1 hunks)docs/prd/vendor-update.md(1 hunks)errors/errors.go(1 hunks)go.mod(1 hunks)internal/exec/mock_vendor_git_interface.go(1 hunks)internal/exec/vendor.go(0 hunks)internal/exec/vendor_diff.go(1 hunks)internal/exec/vendor_diff_integration_test.go(1 hunks)internal/exec/vendor_git_diff.go(1 hunks)internal/exec/vendor_git_diff_test.go(1 hunks)internal/exec/vendor_git_interface.go(1 hunks)internal/exec/vendor_update.go(1 hunks)internal/exec/vendor_update_test.go(1 hunks)internal/exec/vendor_version_check.go(1 hunks)internal/exec/vendor_version_check_test.go(1 hunks)internal/exec/vendor_yaml_updater.go(1 hunks)internal/exec/vendor_yaml_updater_test.go(1 hunks)website/docs/cli/commands/vendor/diff.mdx(1 hunks)website/docs/cli/commands/vendor/update.mdx(1 hunks)
💤 Files with no reviewable changes (1)
- internal/exec/vendor.go
🧰 Additional context used
📓 Path-based instructions (15)
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Unit tests should focus on pure functions and favor table-driven tests.
Test behavior (inputs/outputs), not implementation detail; avoid tautologies and stub-only tests; remove always-skipped tests.
Always call production code within tests rather than duplicating logic under test.
Use t.Skipf with a clear reason when skipping tests; never call t.Skipf without a reason.
Use go.uber.org/mock/mockgen for interface mocks; never hand-write large mock implementations.
Add //go:generate mockgen directives for mocks in test files.
Files:
internal/exec/vendor_git_diff_test.gointernal/exec/vendor_update_test.gointernal/exec/vendor_diff_integration_test.gointernal/exec/vendor_version_check_test.gointernal/exec/vendor_yaml_updater_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods. Applies to single-line, multi-line, inline, and documentation comments.
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines and sort alphabetically within each group; preserve existing aliases.
Add defer perf.Track(...) in all public and critical private functions; include a blank line after the perf.Track call; use "packagename.FunctionName" as the name; pass atmosConfig when available, otherwise nil.
Wrap all returned errors using static errors (from errors/errors.go); use errors.Join to combine errors; use fmt.Errorf with %w to add context; use errors.Is for checks; never rely on err.Error() string matching.
Always use viper.BindEnv with ATMOS_ alternatives; prefer ATMOS_* first with fallback to external env vars when needed.
Prefer many small, focused files; never use //revive:disable:file-length-limit to bypass file length limits.
Use filepath.Join, os.PathSeparator, and runtime.GOOS for portable path/OS handling; use build constraints when necessary.
Files:
internal/exec/vendor_git_diff_test.gointernal/exec/vendor_update_test.gointernal/exec/vendor_diff_integration_test.gointernal/exec/vendor_yaml_updater.gointernal/exec/vendor_update.gointernal/exec/vendor_version_check_test.goerrors/errors.gointernal/exec/vendor_git_diff.gointernal/exec/vendor_diff.gocmd/vendor_update.gointernal/exec/vendor_version_check.gointernal/exec/vendor_yaml_updater_test.gointernal/exec/mock_vendor_git_interface.gointernal/exec/vendor_git_interface.gocmd/vendor_diff.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Use Viper for configuration management and bind environment variables consistently.
Distinguish structured logging from UI output: UI prompts/status/errors to stderr; data/results to stdout; do not use logging for UI elements.
Prefer SDKs over invoking external binaries for cross-platform compatibility (e.g., terraform-exec instead of exec.Command("terraform")).
Files:
internal/exec/vendor_git_diff_test.gointernal/exec/vendor_update_test.gointernal/exec/vendor_diff_integration_test.gointernal/exec/vendor_yaml_updater.gointernal/exec/vendor_update.gointernal/exec/vendor_version_check_test.gointernal/exec/vendor_git_diff.gointernal/exec/vendor_diff.gocmd/vendor_update.gointernal/exec/vendor_version_check.gointernal/exec/vendor_yaml_updater_test.gointernal/exec/mock_vendor_git_interface.gointernal/exec/vendor_git_interface.gocmd/vendor_diff.go
{cmd,internal}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal}/**/*.go: Most text UI must go to stderr (e.g., utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr, ...)); only data/results go to stdout.
Telemetry for commands is automatic via RootCmd.ExecuteC(); for non-standard paths, call telemetry.CaptureCmd/CaptureCmdString explicitly.
Files:
internal/exec/vendor_git_diff_test.gointernal/exec/vendor_update_test.gointernal/exec/vendor_diff_integration_test.gointernal/exec/vendor_yaml_updater.gointernal/exec/vendor_update.gointernal/exec/vendor_version_check_test.gointernal/exec/vendor_git_diff.gointernal/exec/vendor_diff.gocmd/vendor_update.gointernal/exec/vendor_version_check.gointernal/exec/vendor_yaml_updater_test.gointernal/exec/mock_vendor_git_interface.gointernal/exec/vendor_git_interface.gocmd/vendor_diff.go
{pkg,internal,cmd}/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should mirror implementation naming (e.g., foo.go has foo_test.go) and be co-located with implementations.
Files:
internal/exec/vendor_git_diff_test.gointernal/exec/vendor_update_test.gointernal/exec/vendor_diff_integration_test.gointernal/exec/vendor_version_check_test.gointernal/exec/vendor_yaml_updater_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/vendor_yaml_updater.gointernal/exec/vendor_update.goerrors/errors.gointernal/exec/vendor_git_diff.gointernal/exec/vendor_diff.gocmd/vendor_update.gointernal/exec/vendor_version_check.gointernal/exec/mock_vendor_git_interface.gointernal/exec/vendor_git_interface.gocmd/vendor_diff.go
go.{mod,sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
go.{mod,sum}: Manage dependencies with Go modules
Keep dependencies up to date
Files:
go.mod
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use casesAlways build the website (npm run build) after documentation changes to ensure no errors, broken links, or missing assets.
Files:
website/docs/cli/commands/vendor/diff.mdxwebsite/docs/cli/commands/vendor/update.mdx
website/docs/cli/commands/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
All new commands/flags/parameters must have Docusaurus documentation under website/docs/cli/commands//.mdx using the prescribed frontmatter and sections.
Files:
website/docs/cli/commands/vendor/diff.mdxwebsite/docs/cli/commands/vendor/update.mdx
website/**/{*.md,*.mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use definition lists
for arguments and flags in docs instead of tables.
Files:
website/docs/cli/commands/vendor/diff.mdxwebsite/docs/cli/commands/vendor/update.mdx
errors/errors.go
📄 CodeRabbit inference engine (CLAUDE.md)
Define static errors centrally in errors/errors.go (e.g., ErrInvalidComponent, ErrInvalidStack). Do not define dynamic errors inline.
Files:
errors/errors.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriateOne Cobra command per file within cmd/.
Files:
cmd/vendor_update.gocmd/vendor_diff.go
cmd/**
📄 CodeRabbit inference engine (CLAUDE.md)
Embed usage examples via //go:embed and render them using utils.PrintfMarkdown(); follow naming convention atmos___usage.md.
Files:
cmd/vendor_update.gocmd/vendor_diff.go
**/*interface.go
📄 CodeRabbit inference engine (CLAUDE.md)
For internal interfaces, generate mocks with mockgen using -source on the interface file.
Files:
internal/exec/mock_vendor_git_interface.gointernal/exec/vendor_git_interface.go
**/mock_*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Exclude mock_*.go files from coverage metrics.
Files:
internal/exec/mock_vendor_git_interface.go
🧠 Learnings (3)
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to internal/exec/template_funcs.go : Add new template functions in internal/exec/template_funcs.go and register them in the function map with comprehensive tests.
Applied to files:
internal/exec/vendor_update_test.go
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to pkg/git/interface.go : Use mockgen for pkg/git/interface.go to generate mocks (as exemplified).
Applied to files:
internal/exec/mock_vendor_git_interface.gointernal/exec/vendor_git_interface.go
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to pkg/filesystem/interface.go : Use mockgen for pkg/filesystem/interface.go to generate mocks (as exemplified).
Applied to files:
internal/exec/mock_vendor_git_interface.go
🧬 Code graph analysis (11)
internal/exec/vendor_git_diff_test.go (1)
internal/exec/vendor_git_diff.go (1)
GitDiffOptions(19-28)
internal/exec/vendor_diff_integration_test.go (3)
internal/exec/vendor_diff.go (1)
VendorDiffFlags(50-58)internal/exec/mock_vendor_git_interface.go (2)
MockGitOperations(15-18)NewMockGitOperations(26-30)pkg/schema/schema.go (3)
Context(381-396)AtmosConfiguration(27-65)Vendor(748-753)
internal/exec/vendor_yaml_updater.go (3)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (4)
ErrReadFile(91-91)ErrParseFile(12-12)ErrComponentNotFound(388-388)ErrYAMLUpdateFailed(398-398)
internal/exec/vendor_update.go (7)
pkg/perf/perf.go (1)
Track(121-138)internal/exec/cli_utils.go (1)
ProcessCommandLineArgs(76-161)pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (2)
AtmosConfiguration(27-65)Vendor(748-753)pkg/config/const.go (1)
AtmosVendorConfigFileName(53-53)internal/exec/vendor_utils.go (1)
ReadAndProcessVendorConfigFile(76-106)errors/errors.go (2)
ErrVendorConfigNotFound(390-390)ErrNotImplemented(404-404)
internal/exec/vendor_git_diff.go (3)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (3)
ErrCreateTempDir(18-18)ErrGitCommandFailed(77-77)ErrGitDiffFailed(391-391)
internal/exec/vendor_diff.go (8)
pkg/perf/perf.go (1)
Track(121-138)internal/exec/cli_utils.go (1)
ProcessCommandLineArgs(76-161)pkg/config/config.go (1)
InitCliConfig(25-62)errors/errors.go (5)
ErrComponentFlagRequired(389-389)ErrComponentNotFound(388-388)ErrUnsupportedVendorSource(394-394)ErrNoTagsFound(403-403)ErrNotImplemented(404-404)pkg/schema/schema.go (3)
AtmosConfiguration(27-65)Vendor(748-753)AtmosVendorSource(711-720)internal/exec/vendor_git_interface.go (2)
NewGitOperations(26-28)GitOperations(11-20)pkg/config/const.go (1)
AtmosVendorConfigFileName(53-53)internal/exec/vendor_utils.go (1)
ReadAndProcessVendorConfigFile(76-106)
cmd/vendor_update.go (2)
internal/exec/vendor_update.go (1)
ExecuteVendorUpdateCmd(15-40)cmd/cmd_utils.go (1)
ComponentsArgCompletion(768-790)
internal/exec/vendor_yaml_updater_test.go (1)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)
internal/exec/mock_vendor_git_interface.go (1)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)
internal/exec/vendor_git_interface.go (1)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)
cmd/vendor_diff.go (1)
cmd/cmd_utils.go (1)
ComponentsArgCompletion(768-790)
🪛 GitHub Check: Build (linux)
internal/exec/mock_vendor_git_interface.go
[failure] 78-78:
mr.mock.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 69-69:
m.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 63-63:
mr.mock.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 54-54:
m.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 48-48:
mr.mock.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 39-39:
m.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
🪛 GitHub Check: Build (macos)
internal/exec/mock_vendor_git_interface.go
[failure] 78-78:
mr.mock.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 69-69:
m.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 63-63:
mr.mock.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 54-54:
m.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 48-48:
mr.mock.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
[failure] 39-39:
m.ctrl.T undefined (type *"github.com/golang/mock/gomock".Controller has no field or method T)
🪛 LanguageTool
docs/prd/vendor-update.md
[typographical] ~230-~230: Consider using a typographic close quote here.
Context: ...Quote Styles: Single ('), double ("), or unquoted 7. Line Ordering: Ma...
(EN_QUOTES)
[grammar] ~246-~246: Please add a punctuation mark at the end of paragraph.
Context: ...in the Go ecosystem - Battle-tested and maintained Implementation: ```go import "gopk...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~273-~273: Please add a punctuation mark at the end of paragraph.
Context: ...mitations** and file issues upstream if needed Key Requirements: 1. Use `yaml.Nod...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~280-~280: Please add a punctuation mark at the end of paragraph.
Context: ...ering 5. Test with complex anchor/alias scenarios ### Test Cases for YAML Preservation ...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~919-~919: Consider using a typographic opening quote here.
Context: ....yaml | ErrVendorConfigFileNotFound | "Vendor config file not found: vendor.yam...
(EN_QUOTES)
[typographical] ~919-~919: Consider using a typographic close quote here.
Context: ...endor config file not found: vendor.yaml" | Check file path | | Git ls-remote fai...
(EN_QUOTES)
[typographical] ~920-~920: Consider using a typographic opening quote here.
Context: ...emote fails | ErrCheckingForUpdates | "Failed to check updates for vpc: connect...
(EN_QUOTES)
[typographical] ~920-~920: Consider using a typographic close quote here.
Context: ...heck updates for vpc: connection timeout" | Check network, try again | | No tags ...
(EN_QUOTES)
[typographical] ~921-~921: Consider using a typographic opening quote here.
Context: ... | No tags in repo | ErrNoTagsFound | "No version tags found in repository" | U...
(EN_QUOTES)
[typographical] ~921-~921: Consider using a typographic close quote here.
Context: ...` | "No version tags found in repository" | Use commit hash versioning | | Rate l...
(EN_QUOTES)
[typographical] ~922-~922: Consider using a typographic opening quote here.
Context: ...it hit | ErrGitHubRateLimitExceeded | "GitHub rate limit exceeded. Use SSH or s...
(EN_QUOTES)
[typographical] ~922-~922: Consider using a typographic close quote here.
Context: ...t exceeded. Use SSH or set GITHUB_TOKEN." | Wait or authenticate | | Invalid vers...
(EN_QUOTES)
[typographical] ~923-~923: Consider using a typographic opening quote here.
Context: ...format | errUtils.ErrInvalidVersion | "Invalid version format: 'abc'. Expected ...
(EN_QUOTES)
[typographical] ~923-~923: Consider using a typographic close quote here.
Context: ...: 'abc'. Expected semver or commit hash." | Fix version string | | Templated vers...
(EN_QUOTES)
[typographical] ~924-~924: Consider using a typographic opening quote here.
Context: ...| Templated version | (skip silently) | "⚠ vpc (skipped - templated version)" | N...
(EN_QUOTES)
[typographical] ~924-~924: Consider using a typographic close quote here.
Context: ...) | "⚠ vpc (skipped - templated version)" | No action needed | | YAML parse error...
(EN_QUOTES)
[typographical] ~925-~925: Consider using a typographic opening quote here.
Context: ... YAML parse error | yaml.ParseError | "Failed to parse vendor.yaml: line 5, inv...
(EN_QUOTES)
[typographical] ~925-~925: Consider using a typographic close quote here.
Context: ...arse vendor.yaml: line 5, invalid syntax" | Fix YAML syntax | ## Documentation R...
(EN_QUOTES)
[typographical] ~1195-~1195: Consider using a typographic opening quote here.
Context: ... explicit version field - Show warning: "⚠ vpc (skipped - no version field)" - Do...
(EN_QUOTES)
[typographical] ~1195-~1195: Consider using a typographic close quote here.
Context: ...ing: "⚠ vpc (skipped - no version field)" - Don't add version field automatically...
(EN_QUOTES)
[grammar] ~1208-~1208: Please add a punctuation mark at the end of paragraph.
Context: ...xec/vendor_filter.gofor component/tag filtering **Vendor Diff:** - [ ] Createcmd/ven...
(PUNCTUATION_PARAGRAPH_END)
[uncategorized] ~1294-~1294: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...) and documentation requirements ensure high quality and excellent user experience from day ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
website/docs/cli/commands/vendor/diff.mdx
[style] ~186-~186: Consider using the typographical ellipsis character here instead.
Context: ...s (github.com/org/repo) - ✅ Git URLs (git::https://..., git@...) - ✅ HTTPS Git URLs - ❌ OCI...
(ELLIPSIS)
[style] ~186-~186: Consider using the typographical ellipsis character here instead.
Context: ...epo) - ✅ Git URLs (git::https://..., git@...`) - ✅ HTTPS Git URLs - ❌ OCI images (no...
(ELLIPSIS)
website/docs/cli/commands/vendor/update.mdx
[grammar] ~104-~104: Please add a punctuation mark at the end of paragraph.
Context: ...c. - Git commits: Full or short SHA hashes Version comparison uses semantic versi...
(PUNCTUATION_PARAGRAPH_END)
[style] ~116-~116: Consider using the typographical ellipsis character here instead.
Context: ...s (github.com/org/repo) - ✅ Git URLs (git::https://..., git@...) - ✅ HTTPS Git URLs - ❌ OCI...
(ELLIPSIS)
[style] ~116-~116: Consider using the typographical ellipsis character here instead.
Context: ...epo) - ✅ Git URLs (git::https://..., git@...`) - ✅ HTTPS Git URLs - ❌ OCI images (no...
(ELLIPSIS)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/vendor-update.md
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
236-236: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
517-517: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
524-524: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
530-530: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
536-536: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
576-576: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
673-673: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
680-680: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
688-688: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
695-695: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
712-712: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
788-788: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
869-869: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build (windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: autofix
- GitHub Check: Run pre-commit hooks
- GitHub Check: website-deploy-preview
- GitHub Check: Review Dependency Licenses
- GitHub Check: Summary
🔇 Additional comments (20)
go.mod (2)
12-12: Semver direct dependency promotion looks good.Making Masterminds/semver a direct require matches the new version‑resolution logic.
92-92: Review comment is incorrect—cannot remove go.yaml.in/yaml from go.mod without code changes.The codebase has 15+ direct imports of
go.yaml.in/yaml/v3across multiple files (tests/testhelpers/sandbox.go, pkg/validator/schema_validator.go, pkg/schema/schema.go, pkg/filetype/filetype.go, pkg/config/process_yaml.go, internal/exec/, pkg/list/, and others). Removing these entries from go.mod would break the build unless all imports are first consolidated togopkg.in/yaml.v3.The actual concern—mixing both
go.yaml.in/yamlandgopkg.in/yaml—is valid, but the fix requires updating imports in source files first, then cleaning up go.mod.Likely an incorrect or invalid review comment.
internal/exec/vendor_update_test.go (1)
9-54: Solid table tests for splitAndTrim.Covers spacing and empty input; nice.
website/docs/cli/commands/vendor/diff.mdx (2)
1-273: Well-structured docs with clear examples.Looks consistent with the CLI. Nice use of
.
82-101: All defaults verified and consistent; docs accurately reflect implementation.
- ✓
--todefaults to latest semantic version tag (empty string triggers resolution to latest tag in code)- ✓
--unifieddefaults to true (registered in cmd/vendor_diff.go:39)- ✓
--contextdefaults to 3 lines (registered in cmd/vendor_diff.go:36)Help text in
cmd/vendor_diff.gois accurate, and the documented behavior indiff.mdxcorrectly reflects how these defaults work. No changes needed.website/docs/cli/commands/vendor/update.mdx (1)
1-180: Well-structured documentation.The documentation is comprehensive, covering usage, examples, flags, version detection, and supported sources. The examples with output are particularly helpful for users.
internal/exec/vendor_git_diff_test.go (2)
11-96: Comprehensive test coverage for buildGitDiffArgs.The table-driven tests effectively cover color on/off, context length, file filtering, and zero context scenarios.
209-243: The test expectations are correct—no changes needed.The implementation of
stripANSICodesintentionally removes only the escape sequence\x1b[, leaving numeric codes intact (e.g.,"\x1b[31m"becomes"31m"). The test expectations accurately reflect this behavior. The comments in the implementation even acknowledge this is a "simple" approach and suggest regex or a library for more sophisticated stripping.The review comment assumes the function should completely strip all ANSI codes, but the current implementation does partial stripping by design. The tests are correctly verifying that actual behavior.
Likely an incorrect or invalid review comment.
internal/exec/vendor_yaml_updater.go (1)
1-223: Well-implemented YAML updater with proper structure preservation.The implementation correctly uses yaml.v3's Node API to preserve comments, anchors, and formatting. Error handling is consistent with wrapped errors, and performance tracking is properly integrated.
internal/exec/vendor_version_check_test.go (1)
1-229: Excellent test coverage for version utilities.The table-driven tests comprehensively cover semantic version parsing, latest version detection, SHA validation, and Git URI extraction with good edge case handling.
cmd/vendor_diff.go (1)
1-39: Clean command wiring with proper flag setup.The vendor diff command is well-structured with appropriate flags and completion functions. The delegation to
ExecuteVendorDiffCmdmaintains separation of concerns.internal/exec/vendor_update.go (2)
101-147: Placeholder implementation noted.The function contains TODO comments and returns
ErrNotImplemented. This is acceptable for a staged rollout, but ensure the implementation is completed before the PR is merged or clearly document that this is a work-in-progress.Are you planning to complete this implementation in this PR, or is this intentionally being staged for a follow-up?
14-40: Clean command entry point.The function properly initializes the Atmos configuration, parses flags, and delegates to the update executor.
internal/exec/vendor_version_check.go (2)
23-53: Solid Git remote tags fetching.The implementation correctly uses
git ls-remotewith a timeout context and properly parses the tag output.
1-119: No compilation issues; review comment is incorrect.The
extractGitURIfunction exists ininternal/exec/vendor_diff.go(line 231). Since bothvendor_diff.goandvendor_version_check_test.goare in the sameinternal/execpackage, the test file can access the unexported function without issues. The code compiles and functions as intended.Likely an incorrect or invalid review comment.
cmd/vendor_update.go (1)
1-39: Well-structured vendor update command.The command setup is clean with appropriate flags, defaults, and completion functions. The delegation to
ExecuteVendorUpdateCmdmaintains proper separation between CLI and business logic.internal/exec/mock_vendor_git_interface.go (1)
37-50: The review comment is incorrect. The T.Helper() pattern in generated mocks has been intentional since v1.2.0 to better direct the callstack, and the Controller struct in gomock has a T field that should only be called within generated mocks. The library and mockgen versions should be kept in sync, and with gomock v1.6.0 pinned in go.mod, the generated mock code with T.Helper() calls is valid and correct.The mock file compiles without errors when gomock v1.6.0 is properly imported. No changes to the mock code are needed.
internal/exec/vendor_diff.go (1)
168-176: Fix missing period on line 245; no changes needed to .git stripping logic.The
.gitsuffix stripping is intentional and works correctly. Git operations likegit ls-remoteandgit fetchhandle URLs with or without.gittransparently, so the stripped URI works as expected. Tests confirm this is designed behavior.The only issue is the comment on line 245 missing a period, per coding guidelines:
// Clean up .git suffix if present.internal/exec/vendor_git_interface.go (2)
1-7: Clean interface setup with proper mock generation.The go:generate directive correctly configures mockgen for interface mocking, and the import structure is appropriate for this focused interface file.
9-20: Well-designed interface with clear separation of concerns.The three methods provide a focused API for Git operations needed by vendor commands. Documentation is clear and complete.
| @@ -0,0 +1,230 @@ | |||
| package exec | |||
There was a problem hiding this comment.
🧩 Analysis chain
Add a //go:generate mockgen directive for GitOperations.
Place a directive in the interface file or here to regenerate mocks consistently.
Example (adjust paths/types to your repo):
//go:generate mockgen -destination=internal/exec/mock_vendor_git_interface.go -package=exec github.com/cloudposse/atmos/internal/exec GitOperations🏁 Script executed:
#!/bin/bash
# Check for existing mockgen directives.
rg -n "//go:generate mockgen" internal/exec -C2Length of output: 3101
I'll verify the current state of the interface file and test file to determine if the directive is already in place.
🏁 Script executed:
#!/bin/bash
# Check vendor_git_interface.go for GitOperations definition
echo "=== vendor_git_interface.go ==="
cat internal/exec/vendor_git_interface.go
echo -e "\n=== vendor_diff_integration_test.go ==="
cat internal/exec/vendor_diff_integration_test.go 2>/dev/null || echo "File not found or is very short"
echo -e "\n=== Check for mock generation files ==="
fd "mock_.*vendor.*" internal/exec/Length of output: 7774
Update imports and add mockgen directive to the test file.
The //go:generate mockgen directive already exists in vendor_git_interface.go (line 3) and mocks are generating correctly. However, per the project's patterns and learnings, the test file should have its own directive, and the import needs updating to the modern standard.
Changes needed:
- Replace
github.com/golang/mock/gomockwithgo.uber.org/mock/gomockin vendor_diff_integration_test.go - Add
//go:generate mockgendirective to vendor_diff_integration_test.go (line 3) matching the pattern in version_test.go and describe_affected_utils_test.go
Example:
//go:generate mockgen -destination=mock_git_operations_test.go -package=exec github.com/cloudposse/atmos/internal/exec GitOperations🤖 Prompt for AI Agents
In internal/exec/vendor_diff_integration_test.go around line 1, update the
imports to replace "github.com/golang/mock/gomock" with
"go.uber.org/mock/gomock" and add a //go:generate mockgen directive at line 3
matching the project pattern (for example: //go:generate mockgen
-destination=mock_git_operations_test.go -package=exec
github.com/cloudposse/atmos/internal/exec GitOperations), ensuring the directive
and import are present so tests generate mocks consistently.
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
- Add `constraints` field to vendor config schema - Implement constraint resolution logic with: - Semver range constraints (^, ~, >=, <, etc.) - Excluded versions list with wildcard support - Pre-release filtering (no_prereleases flag) - Add comprehensive test coverage for all constraint scenarios - Update PRD with version constraints specification - Update JSON schema for vendor package 1.0 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Clarify that vendor diff is GitHub-only in v1 (uses Compare API) - Document that non-GitHub sources return "not implemented" - Replace invalid `git diff remote-url#ref` syntax with correct temporary bare repo workflow for future non-GitHub support - Update backward compatibility section to reflect actual implementation status instead of "reserved for future" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (8)
docs/prd/vendor-update.md (1)
1-1485: Minor formatting issues throughout—acceptable for internal PRD.Static analysis flagged ~35 minor formatting issues: missing language specs on code blocks (e.g., lines 65, 767, 864), quote style inconsistencies, missing trailing punctuation, and emphasis used instead of markdown headings. These are editorial refinements that don't affect implementation readiness or technical clarity. Consider consolidating these fixes in a separate pass if the document will be shared externally or published to docs.
pkg/schema/schema.go (1)
712-721: Addomitemptyto Tags for cleaner marshaling.Looks good. Consider
yaml:",omitempty" json:",omitempty"onTagsto avoid emitting empty arrays and reduce churn in vendor.yaml.- Tags []string `yaml:"tags" json:"tags" mapstructure:"tags"` + Tags []string `yaml:"tags,omitempty" json:"tags,omitempty" mapstructure:"tags"`pkg/datafetcher/schema/vendor/package/1.0.json (1)
69-89: Harden the constraints schema.
- Disallow unknown keys.
- Prevent empty constraint strings.
"constraints": { "type": "object", "description": "Version constraints for vendor updates", + "additionalProperties": false, "properties": { "version": { "type": "string", "description": "Semantic version constraint (e.g., ^1.0.0, ~1.2.0, >=1.0.0 <2.0.0)" + ,"minLength": 1 }, "excluded_versions": { "type": "array", "description": "List of versions to exclude (supports wildcards like 1.5.*)", "items": { "type": "string" + ,"minLength": 1 } }, "no_prereleases": { "type": "boolean", "description": "If true, exclude pre-release versions (alpha, beta, rc)" } } },internal/exec/vendor_version_constraints_test.go (1)
12-140: Assert sentinel errors with errors.Is.Strengthen failure-path tests by checking specific sentinels (e.g., ErrNoVersionsAvailable, ErrNoVersionsMatchConstraints, ErrInvalidSemverConstraint) instead of only wantErr.
@@ -import ( - "testing" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/cloudposse/atmos/pkg/schema" -) +import ( + "errors" + "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + errUtils "github.com/cloudposse/atmos/errors" + "github.com/cloudposse/atmos/pkg/schema" +) @@ - if tt.wantErr { - require.Error(t, err) - } else { + if tt.wantErr { + require.Error(t, err) + switch tt.name { + case "no versions match constraints": + assert.ErrorIs(t, err, errUtils.ErrNoVersionsMatchConstraints) + case "empty version list": + assert.ErrorIs(t, err, errUtils.ErrNoVersionsAvailable) + case "invalid semver constraint": + assert.True(t, errors.Is(err, errUtils.ErrInvalidSemverConstraint)) + } + } else { require.NoError(t, err) assert.Equal(t, tt.want, got) }Also applies to: 353-409
internal/exec/vendor_source_provider_test.go (1)
9-81: Nice coverage on provider routing. Add a couple more source forms.Consider adding cases for:
- ssh://[email protected]/cloudposse/terraform-aws-vpc.git
- https://github.com/cloudposse/terraform-aws-vpc?ref=refs/tags/v1.0.0 (slashes in ref)
internal/exec/vendor_source_provider_unsupported.go (1)
19-27: Optionally track other unsupported operations.Low overhead; helps trace usage.
func (u *UnsupportedSourceProvider) GetAvailableVersions(source string) ([]string, error) { + defer perf.Track(nil, "exec.UnsupportedSourceProvider.GetAvailableVersions")() return nil, fmt.Errorf("%w: version listing not supported for this source type", errUtils.ErrUnsupportedVendorSource) } @@ func (u *UnsupportedSourceProvider) VerifyVersion(source string, version string) (bool, error) { + defer perf.Track(nil, "exec.UnsupportedSourceProvider.VerifyVersion")() return false, fmt.Errorf("%w: version verification not supported for this source type", errUtils.ErrUnsupportedVendorSource) }internal/exec/vendor_source_provider_github.go (2)
30-41: Track List/Verify for consistency.Add perf tracking to small public methods.
func (g *GitHubSourceProvider) GetAvailableVersions(source string) ([]string, error) { + defer perf.Track(nil, "exec.GitHubSourceProvider.GetAvailableVersions")() + // Use existing Git operations to get tags gitURI := extractGitURI(source) return getGitRemoteTags(gitURI) } @@ func (g *GitHubSourceProvider) VerifyVersion(source string, version string) (bool, error) { + defer perf.Track(nil, "exec.GitHubSourceProvider.VerifyVersion")() + gitURI := extractGitURI(source) return checkGitRef(gitURI, version) }
176-199: Handle non-200 in rate limit check.Add status check to avoid decoding error bodies as JSON.
func (g *GitHubSourceProvider) CheckGitHubRateLimit() (*GitHubRateLimitResponse, error) { @@ resp, err := g.httpClient.Do(req) if err != nil { return nil, err } defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + b, _ := io.ReadAll(resp.Body) + return nil, fmt.Errorf("%w: GitHub API returned status %d: %s", + errUtils.ErrGitDiffFailed, resp.StatusCode, string(b)) + } + var rateLimit GitHubRateLimitResponse if err := json.NewDecoder(resp.Body).Decode(&rateLimit); err != nil { return nil, err }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
docs/prd/vendor-update.md(1 hunks)errors/errors.go(1 hunks)internal/exec/vendor_source_provider.go(1 hunks)internal/exec/vendor_source_provider_git.go(1 hunks)internal/exec/vendor_source_provider_github.go(1 hunks)internal/exec/vendor_source_provider_test.go(1 hunks)internal/exec/vendor_source_provider_unsupported.go(1 hunks)internal/exec/vendor_version_constraints.go(1 hunks)internal/exec/vendor_version_constraints_test.go(1 hunks)pkg/datafetcher/schema/vendor/package/1.0.json(1 hunks)pkg/schema/schema.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- errors/errors.go
🧰 Additional context used
📓 Path-based instructions (8)
pkg/datafetcher/schema/vendor/package/1.0.json
📄 CodeRabbit inference engine (CLAUDE.md)
Update vendor package schema when adding vendor-related configuration options.
Files:
pkg/datafetcher/schema/vendor/package/1.0.json
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods. Applies to single-line, multi-line, inline, and documentation comments.
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines and sort alphabetically within each group; preserve existing aliases.
Add defer perf.Track(...) in all public and critical private functions; include a blank line after the perf.Track call; use "packagename.FunctionName" as the name; pass atmosConfig when available, otherwise nil.
Wrap all returned errors using static errors (from errors/errors.go); use errors.Join to combine errors; use fmt.Errorf with %w to add context; use errors.Is for checks; never rely on err.Error() string matching.
Always use viper.BindEnv with ATMOS_ alternatives; prefer ATMOS_* first with fallback to external env vars when needed.
Prefer many small, focused files; never use //revive:disable:file-length-limit to bypass file length limits.
Use filepath.Join, os.PathSeparator, and runtime.GOOS for portable path/OS handling; use build constraints when necessary.
Files:
internal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_version_constraints.gointernal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_test.gointernal/exec/vendor_source_provider_github.gointernal/exec/vendor_version_constraints_test.gopkg/schema/schema.gointernal/exec/vendor_source_provider_git.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_version_constraints.gointernal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_github.gopkg/schema/schema.gointernal/exec/vendor_source_provider_git.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Use Viper for configuration management and bind environment variables consistently.
Distinguish structured logging from UI output: UI prompts/status/errors to stderr; data/results to stdout; do not use logging for UI elements.
Prefer SDKs over invoking external binaries for cross-platform compatibility (e.g., terraform-exec instead of exec.Command("terraform")).
Files:
internal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_version_constraints.gointernal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_test.gointernal/exec/vendor_source_provider_github.gointernal/exec/vendor_version_constraints_test.gopkg/schema/schema.gointernal/exec/vendor_source_provider_git.go
{cmd,internal}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal}/**/*.go: Most text UI must go to stderr (e.g., utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr, ...)); only data/results go to stdout.
Telemetry for commands is automatic via RootCmd.ExecuteC(); for non-standard paths, call telemetry.CaptureCmd/CaptureCmdString explicitly.
Files:
internal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_version_constraints.gointernal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_test.gointernal/exec/vendor_source_provider_github.gointernal/exec/vendor_version_constraints_test.gointernal/exec/vendor_source_provider_git.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Unit tests should focus on pure functions and favor table-driven tests.
Test behavior (inputs/outputs), not implementation detail; avoid tautologies and stub-only tests; remove always-skipped tests.
Always call production code within tests rather than duplicating logic under test.
Use t.Skipf with a clear reason when skipping tests; never call t.Skipf without a reason.
Use go.uber.org/mock/mockgen for interface mocks; never hand-write large mock implementations.
Add //go:generate mockgen directives for mocks in test files.
Files:
internal/exec/vendor_source_provider_test.gointernal/exec/vendor_version_constraints_test.go
{pkg,internal,cmd}/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should mirror implementation naming (e.g., foo.go has foo_test.go) and be co-located with implementations.
Files:
internal/exec/vendor_source_provider_test.gointernal/exec/vendor_version_constraints_test.go
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/schema/schema.go
🧠 Learnings (2)
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to pkg/datafetcher/schema/vendor/package/1.0.json : Update vendor package schema when adding vendor-related configuration options.
Applied to files:
pkg/datafetcher/schema/vendor/package/1.0.json
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to pkg/datafetcher/schema/atmos/manifest/1.0.json : Update schema for atmos manifest when introducing new configuration options.
Applied to files:
pkg/datafetcher/schema/vendor/package/1.0.json
🧬 Code graph analysis (7)
internal/exec/vendor_source_provider_unsupported.go (3)
internal/exec/vendor_source_provider.go (2)
VendorSourceProvider(11-24)SourceOperation(27-27)errors/errors.go (1)
ErrUnsupportedVendorSource(394-394)pkg/schema/schema.go (1)
AtmosConfiguration(27-65)
internal/exec/vendor_version_constraints.go (2)
pkg/schema/schema.go (1)
VendorConstraints(724-728)errors/errors.go (3)
ErrNoVersionsAvailable(405-405)ErrNoVersionsMatchConstraints(406-406)ErrInvalidSemverConstraint(407-407)
internal/exec/vendor_source_provider.go (4)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)internal/exec/vendor_source_provider_github.go (1)
NewGitHubSourceProvider(22-28)internal/exec/vendor_source_provider_git.go (1)
NewGenericGitSourceProvider(16-18)internal/exec/vendor_source_provider_unsupported.go (1)
NewUnsupportedSourceProvider(15-17)
internal/exec/vendor_source_provider_test.go (4)
internal/exec/vendor_source_provider.go (6)
SourceOperation(27-27)OperationGetDiff(37-37)OperationListVersions(31-31)GetProviderForSource(44-58)OperationVerifyVersion(34-34)OperationFetchSource(40-40)internal/exec/vendor_source_provider_github.go (2)
GitHubSourceProvider(17-19)NewGitHubSourceProvider(22-28)internal/exec/vendor_source_provider_git.go (2)
GenericGitSourceProvider(13-13)NewGenericGitSourceProvider(16-18)internal/exec/vendor_source_provider_unsupported.go (2)
UnsupportedSourceProvider(12-12)NewUnsupportedSourceProvider(15-17)
internal/exec/vendor_source_provider_github.go (4)
internal/exec/vendor_source_provider.go (6)
VendorSourceProvider(11-24)SourceOperation(27-27)OperationListVersions(31-31)OperationVerifyVersion(34-34)OperationGetDiff(37-37)OperationFetchSource(40-40)pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (2)
ErrGitDiffFailed(391-391)ErrParseURL(13-13)
internal/exec/vendor_version_constraints_test.go (1)
pkg/schema/schema.go (1)
VendorConstraints(724-728)
internal/exec/vendor_source_provider_git.go (3)
internal/exec/vendor_source_provider.go (6)
VendorSourceProvider(11-24)SourceOperation(27-27)OperationListVersions(31-31)OperationVerifyVersion(34-34)OperationFetchSource(40-40)OperationGetDiff(37-37)pkg/schema/schema.go (1)
AtmosConfiguration(27-65)errors/errors.go (1)
ErrNotImplemented(404-404)
🪛 LanguageTool
docs/prd/vendor-update.md
[grammar] ~227-~227: Please add a punctuation mark at the end of paragraph.
Context: ...r constraints familiar from npm, cargo, composer ### Configuration Schema Constraints ...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~265-~265: Consider using a typographic close quote here.
Context: ...ration. Supports: - Exact versions: "1.2.3" - exclude this specific version ...
(EN_QUOTES)
[typographical] ~266-~266: Consider using a typographic close quote here.
Context: ...ecific version - Wildcard patterns: "1.5.*" - exclude all 1.5.x versions - *...
(EN_QUOTES)
[typographical] ~277-~277: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...se versions (alpha, beta, rc, pre, etc.). **Examples of excluded versions when `...
(WRB_QUESTION_MARK)
[uncategorized] ~293-~293: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ...ons** - skip non-semver tags 3. Apply no_prereleases filter - remove alpha/beta/rc if ena...
(EN_WORD_COHERENCY)
[typographical] ~409-~409: Consider using a typographic close quote here.
Context: ...Quote Styles: Single ('), double ("), or unquoted 7. Line Ordering: Ma...
(EN_QUOTES)
[grammar] ~425-~425: Please add a punctuation mark at the end of paragraph.
Context: ...in the Go ecosystem - Battle-tested and maintained Implementation: ```go import "gopk...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~452-~452: Please add a punctuation mark at the end of paragraph.
Context: ...mitations** and file issues upstream if needed Key Requirements: 1. Use `yaml.Nod...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~459-~459: Please add a punctuation mark at the end of paragraph.
Context: ...ering 5. Test with complex anchor/alias scenarios ### Test Cases for YAML Preservation ...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~629-~629: Consider using typographic quotation marks here.
Context: ...itLab, Bitbucket, self-hosted), returns "not implemented" - Not applicable to OCI registries, loc...
(EN_QUOTES)
[typographical] ~1110-~1110: Consider using a typographic opening quote here.
Context: ....yaml | ErrVendorConfigFileNotFound | "Vendor config file not found: vendor.yam...
(EN_QUOTES)
[typographical] ~1110-~1110: Consider using a typographic close quote here.
Context: ...endor config file not found: vendor.yaml" | Check file path | | Git ls-remote fai...
(EN_QUOTES)
[typographical] ~1111-~1111: Consider using a typographic opening quote here.
Context: ...emote fails | ErrCheckingForUpdates | "Failed to check updates for vpc: connect...
(EN_QUOTES)
[typographical] ~1111-~1111: Consider using a typographic close quote here.
Context: ...heck updates for vpc: connection timeout" | Check network, try again | | No tags ...
(EN_QUOTES)
[typographical] ~1112-~1112: Consider using a typographic opening quote here.
Context: ... | No tags in repo | ErrNoTagsFound | "No version tags found in repository" | U...
(EN_QUOTES)
[typographical] ~1112-~1112: Consider using a typographic close quote here.
Context: ...` | "No version tags found in repository" | Use commit hash versioning | | Rate l...
(EN_QUOTES)
[typographical] ~1113-~1113: Consider using a typographic opening quote here.
Context: ...it hit | ErrGitHubRateLimitExceeded | "GitHub rate limit exceeded. Use SSH or s...
(EN_QUOTES)
[typographical] ~1113-~1113: Consider using a typographic close quote here.
Context: ...t exceeded. Use SSH or set GITHUB_TOKEN." | Wait or authenticate | | Invalid vers...
(EN_QUOTES)
[typographical] ~1114-~1114: Consider using a typographic opening quote here.
Context: ...format | errUtils.ErrInvalidVersion | "Invalid version format: 'abc'. Expected ...
(EN_QUOTES)
[typographical] ~1114-~1114: Consider using a typographic close quote here.
Context: ...: 'abc'. Expected semver or commit hash." | Fix version string | | Templated vers...
(EN_QUOTES)
[typographical] ~1115-~1115: Consider using a typographic opening quote here.
Context: ...| Templated version | (skip silently) | "⚠ vpc (skipped - templated version)" | N...
(EN_QUOTES)
[typographical] ~1115-~1115: Consider using a typographic close quote here.
Context: ...) | "⚠ vpc (skipped - templated version)" | No action needed | | YAML parse error...
(EN_QUOTES)
[typographical] ~1116-~1116: Consider using a typographic opening quote here.
Context: ... YAML parse error | yaml.ParseError | "Failed to parse vendor.yaml: line 5, inv...
(EN_QUOTES)
[typographical] ~1116-~1116: Consider using a typographic close quote here.
Context: ...arse vendor.yaml: line 5, invalid syntax" | Fix YAML syntax | ## Documentation R...
(EN_QUOTES)
[typographical] ~1309-~1309: Consider using typographic quotation marks here.
Context: ...ons using GitHub's Compare API; returns "not implemented" for non-GitHub sources ## Migration Pa...
(EN_QUOTES)
[typographical] ~1386-~1386: Consider using a typographic opening quote here.
Context: ... explicit version field - Show warning: "⚠ vpc (skipped - no version field)" - Do...
(EN_QUOTES)
[typographical] ~1386-~1386: Consider using a typographic close quote here.
Context: ...ing: "⚠ vpc (skipped - no version field)" - Don't add version field automatically...
(EN_QUOTES)
[grammar] ~1399-~1399: Please add a punctuation mark at the end of paragraph.
Context: ...xec/vendor_filter.gofor component/tag filtering **Vendor Diff:** - [ ] Createcmd/ven...
(PUNCTUATION_PARAGRAPH_END)
[uncategorized] ~1485-~1485: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...) and documentation requirements ensure high quality and excellent user experience from day ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/vendor-update.md
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
415-415: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
708-708: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
715-715: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
721-721: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
727-727: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
767-767: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
864-864: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
871-871: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
879-879: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
886-886: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
903-903: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
979-979: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1060-1060: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build (windows)
- GitHub Check: Review Dependency Licenses
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: autofix
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (8)
docs/prd/vendor-update.md (4)
1114-1114: Verify missing error definition.Line 1114 references
errUtils.ErrInvalidVersionin the error scenarios table, but this error type isn't defined in the error list at lines 1094–1103. Confirm whether this error should be added to the error definitions or if the table entry should reference a different error.
399-460: YAML preservation strategy is well-founded.The documented approach (gopkg.in/yaml.v3 with yaml.Node API) is sound and battle-tested. The explicit guidance against writing a custom YAML parser and the detailed test cases (lines 461–509) provide solid guardrails for implementation. This significantly reduces risk of YAML corruption during updates.
220-397: Version constraints design is thorough.The constraint system (semver ranges, exclusions, pre-release filtering) covers typical real-world scenarios. Schema updates at lines 346–367 and JSON schema updates at lines 369–397 are clearly specified. The examples (lines 301–344) effectively demonstrate the constraint patterns. Implementation path is clear.
605-625: Color handling logic is clearly specified.The color decision tree (lines 607–625) is explicit and handles TTY detection, environment flags, and
TERM=dumbcorrectly. This is solid guidance for avoiding color escapes in pipes/CI environments while maintaining visual feedback for interactive use.pkg/schema/schema.go (1)
723-728: VendorConstraints type LGTM.Shape aligns with usage and JSON schema.
internal/exec/vendor_source_provider_git.go (3)
3-9: Import organization looks good.Properly grouped stdlib and Atmos packages with correct separation.
11-13: Type definition is clean.The stateless struct design is appropriate for this provider.
60-67: The review comment concern is incorrect.The
isGitSourcefunction is actively used byGetProviderForSourceininternal/exec/vendor_source_provider.go(line 52) and has test coverage ininternal/exec/vendor_source_provider_test.go(line 273). It's not dead code.Likely an incorrect or invalid review comment.
pkg/vendoring/source_provider_git.go
Outdated
| // GetAvailableVersions implements VendorSourceProvider.GetAvailableVersions. | ||
| func (g *GenericGitSourceProvider) GetAvailableVersions(source string) ([]string, error) { | ||
| gitURI := extractGitURI(source) | ||
| return getGitRemoteTags(gitURI) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Add performance tracking and consider error context.
Missing defer perf.Track(...) call per guidelines. Also, errors from getGitRemoteTags should be wrapped with context about the source being queried.
As per coding guidelines.
Apply this diff:
func (g *GenericGitSourceProvider) GetAvailableVersions(source string) ([]string, error) {
+ defer perf.Track(nil, "exec.GenericGitSourceProvider.GetAvailableVersions")()
+
gitURI := extractGitURI(source)
- return getGitRemoteTags(gitURI)
+ tags, err := getGitRemoteTags(gitURI)
+ if err != nil {
+ return nil, fmt.Errorf("failed to get available versions from %s: %w", source, err)
+ }
+ return tags, nil
}Verify that extractGitURI and getGitRemoteTags exist and return appropriate types:
🏁 Script executed:
#!/bin/bash
# Search for extractGitURI and getGitRemoteTags function definitions
rg -nP --type=go -C3 'func (extractGitURI|getGitRemoteTags)\b'Length of output: 953
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full context of vendor_source_provider_git.go to see imports and other methods
cat internal/exec/vendor_source_provider_git.go
Length of output: 2183
🏁 Script executed:
#!/bin/bash
# Search for perf.Track usage patterns in similar vendor files
rg -nP --type=go 'defer perf\.Track' internal/exec/ | head -20Length of output: 1989
🏁 Script executed:
#!/bin/bash
# Check for fmt.Errorf patterns in the file
rg -nP --type=go 'fmt\.Errorf.*%w' internal/exec/vendor_source_provider_git.goLength of output: 117
The suggested diff is incomplete and would fail to compile; also incomplete scope for perf.Track.
The review flagged GetAvailableVersions correctly but the suggested fix has two critical gaps:
-
Missing import: The
perfpackage is not imported invendor_source_provider_git.go. The diff must add"github.com/cloudposse/atmos/pkg/perf"to imports. -
Incomplete coverage: Per guidelines, all public methods need
defer perf.Track(...). The file has four public methods—GetAvailableVersions,VerifyVersion,GetDiff, andSupportsOperation—but only one was flagged. At minimum, the three methods with business logic (GetAvailableVersions,VerifyVersion,GetDiff) should be covered consistently.
The error wrapping suggestion for GetAvailableVersions is valid; the file already uses the correct fmt.Errorf(..., %w, ...) pattern in GetDiff (line 45).
🤖 Prompt for AI Agents
internal/exec/vendor_source_provider_git.go lines 20-24: the proposed change to
GetAvailableVersions omitted adding the perf import and failed to add perf.Track
instrumentation to other public methods; update imports to include
"github.com/cloudposse/atmos/pkg/perf", modify GetAvailableVersions to defer
perf.Track(...) at start and wrap returned errors with fmt.Errorf(..., %w), and
also add defer perf.Track(...) to VerifyVersion and GetDiff (the three methods
with business logic) so all public operations are consistently tracked.
pkg/vendoring/source_provider_git.go
Outdated
| // VerifyVersion implements VendorSourceProvider.VerifyVersion. | ||
| func (g *GenericGitSourceProvider) VerifyVersion(source string, version string) (bool, error) { | ||
| gitURI := extractGitURI(source) | ||
| return checkGitRef(gitURI, version) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add performance tracking and error context.
Same issues as GetAvailableVersions: missing defer perf.Track(...) and errors should be wrapped with context.
As per coding guidelines.
Apply this diff:
func (g *GenericGitSourceProvider) VerifyVersion(source string, version string) (bool, error) {
+ defer perf.Track(nil, "exec.GenericGitSourceProvider.VerifyVersion")()
+
gitURI := extractGitURI(source)
- return checkGitRef(gitURI, version)
+ exists, err := checkGitRef(gitURI, version)
+ if err != nil {
+ return false, fmt.Errorf("failed to verify version %s in %s: %w", version, source, err)
+ }
+ return exists, nil
}📝 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.
| // VerifyVersion implements VendorSourceProvider.VerifyVersion. | |
| func (g *GenericGitSourceProvider) VerifyVersion(source string, version string) (bool, error) { | |
| gitURI := extractGitURI(source) | |
| return checkGitRef(gitURI, version) | |
| } | |
| // VerifyVersion implements VendorSourceProvider.VerifyVersion. | |
| func (g *GenericGitSourceProvider) VerifyVersion(source string, version string) (bool, error) { | |
| defer perf.Track(nil, "exec.GenericGitSourceProvider.VerifyVersion")() | |
| gitURI := extractGitURI(source) | |
| exists, err := checkGitRef(gitURI, version) | |
| if err != nil { | |
| return false, fmt.Errorf("failed to verify version %s in %s: %w", version, source, err) | |
| } | |
| return exists, nil | |
| } |
🤖 Prompt for AI Agents
In internal/exec/vendor_source_provider_git.go around lines 26-30, add
performance tracking and wrap errors with context: create a context (e.g., ctx
:= context.Background()) and add defer perf.Track(ctx,
"GenericGitSourceProvider.VerifyVersion") at the top of the function; call
checkGitRef with captured results (ok, err := checkGitRef(gitURI, version)) and
if err != nil return false, fmt.Errorf("failed to verify git ref %s for source
%s: %w", version, gitURI, err), otherwise return ok, nil so all errors are
wrapped with contextual information.
pkg/vendoring/source_provider_git.go
Outdated
| // GetDiff implements VendorSourceProvider.GetDiff. | ||
| // For generic Git providers, diff functionality is not implemented. | ||
| // | ||
| //nolint:revive // Seven parameters needed for interface compatibility. | ||
| func (g *GenericGitSourceProvider) GetDiff( | ||
| atmosConfig *schema.AtmosConfiguration, | ||
| source string, | ||
| fromVersion string, | ||
| toVersion string, | ||
| filePath string, | ||
| contextLines int, | ||
| noColor bool, | ||
| ) ([]byte, error) { | ||
| return nil, fmt.Errorf("%w: diff functionality is only supported for GitHub sources", errUtils.ErrNotImplemented) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add performance tracking; implementation is correct.
The not-implemented approach for non-GitHub sources is appropriate. However, add defer perf.Track(...) per guidelines.
As per coding guidelines.
Apply this diff:
func (g *GenericGitSourceProvider) GetDiff(
atmosConfig *schema.AtmosConfiguration,
source string,
fromVersion string,
toVersion string,
filePath string,
contextLines int,
noColor bool,
) ([]byte, error) {
+ defer perf.Track(atmosConfig, "exec.GenericGitSourceProvider.GetDiff")()
+
return nil, fmt.Errorf("%w: diff functionality is only supported for GitHub sources", errUtils.ErrNotImplemented)
}🤖 Prompt for AI Agents
In internal/exec/vendor_source_provider_git.go around lines 32 to 46, add
performance tracking to the GetDiff function by inserting a deferred
perf.Track(...) call at the start of the function following project conventions
(use the standard operation name like "GenericGitSourceProvider.GetDiff" and the
required parameters for perf.Track in this repo), and ensure the perf package is
imported; keep the existing return error behavior unchanged.
pkg/vendoring/source_provider_git.go
Outdated
| // SupportsOperation implements VendorSourceProvider.SupportsOperation. | ||
| func (g *GenericGitSourceProvider) SupportsOperation(operation SourceOperation) bool { | ||
| switch operation { | ||
| case OperationListVersions, OperationVerifyVersion, OperationFetchSource: | ||
| return true | ||
| case OperationGetDiff: | ||
| return false // Not implemented for generic Git | ||
| default: | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
Add performance tracking and fix comment punctuation.
Missing defer perf.Track(...) and the inline comment at line 54 needs a period per guidelines.
As per coding guidelines.
Apply this diff:
func (g *GenericGitSourceProvider) SupportsOperation(operation SourceOperation) bool {
+ defer perf.Track(nil, "exec.GenericGitSourceProvider.SupportsOperation")()
+
switch operation {
case OperationListVersions, OperationVerifyVersion, OperationFetchSource:
return true
case OperationGetDiff:
- return false // Not implemented for generic Git
+ return false // Not implemented for generic Git.
default:
return false
}
}📝 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.
| // SupportsOperation implements VendorSourceProvider.SupportsOperation. | |
| func (g *GenericGitSourceProvider) SupportsOperation(operation SourceOperation) bool { | |
| switch operation { | |
| case OperationListVersions, OperationVerifyVersion, OperationFetchSource: | |
| return true | |
| case OperationGetDiff: | |
| return false // Not implemented for generic Git | |
| default: | |
| return false | |
| } | |
| } | |
| // SupportsOperation implements VendorSourceProvider.SupportsOperation. | |
| func (g *GenericGitSourceProvider) SupportsOperation(operation SourceOperation) bool { | |
| defer perf.Track(nil, "exec.GenericGitSourceProvider.SupportsOperation")() | |
| switch operation { | |
| case OperationListVersions, OperationVerifyVersion, OperationFetchSource: | |
| return true | |
| case OperationGetDiff: | |
| return false // Not implemented for generic Git. | |
| default: | |
| return false | |
| } | |
| } |
🤖 Prompt for AI Agents
In internal/exec/vendor_source_provider_git.go around lines 48 to 58, add
performance tracking by invoking defer perf.Track(ctx,
"GenericGitSourceProvider.SupportsOperation") (or the appropriate context/label
used in the project) at the start of the method so execution time is recorded,
and update the inline comment on the OperationGetDiff case to end with a period
(change "Not implemented for generic Git" to "Not implemented for generic
Git."). Ensure you import or reference the perf package consistently and that
ctx (or the correct context variable) is available in scope for the defer call.
| // Use GitHub Compare API | ||
| // https://docs.github.com/en/rest/commits/commits#compare-two-commits | ||
| compareURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/compare/%s...%s", | ||
| owner, repo, fromVersion, toVersion) | ||
|
|
||
| req, err := http.NewRequest("GET", compareURL, nil) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("%w: failed to create request: %s", errUtils.ErrGitDiffFailed, err) | ||
| } |
There was a problem hiding this comment.
Escape ref names in Compare API URL.
Refs may include slashes; path-escape both ends.
- compareURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/compare/%s...%s",
- owner, repo, fromVersion, toVersion)
+ compareURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/compare/%s...%s",
+ owner, repo, url.PathEscape(fromVersion), url.PathEscape(toVersion))Add import:
import (
"encoding/json"
"fmt"
"io"
"net/http"
+ "net/url"
"strings"
"time"📝 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.
| // Use GitHub Compare API | |
| // https://docs.github.com/en/rest/commits/commits#compare-two-commits | |
| compareURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/compare/%s...%s", | |
| owner, repo, fromVersion, toVersion) | |
| req, err := http.NewRequest("GET", compareURL, nil) | |
| if err != nil { | |
| return nil, fmt.Errorf("%w: failed to create request: %s", errUtils.ErrGitDiffFailed, err) | |
| } | |
| // Use GitHub Compare API | |
| // https://docs.github.com/en/rest/commits/commits#compare-two-commits | |
| compareURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/compare/%s...%s", | |
| owner, repo, url.PathEscape(fromVersion), url.PathEscape(toVersion)) | |
| req, err := http.NewRequest("GET", compareURL, nil) | |
| if err != nil { | |
| return nil, fmt.Errorf("%w: failed to create request: %s", errUtils.ErrGitDiffFailed, err) | |
| } |
🤖 Prompt for AI Agents
In internal/exec/vendor_source_provider_github.go around lines 63 to 71, the
Compare API URL uses raw ref names which may contain slashes; path-escape the
ref names before formatting the URL (e.g., use url.PathEscape on fromVersion and
toVersion) and update the fmt.Sprintf call to use the escaped values, and add
the necessary import ("net/url") to the file.
| // getGitHubToken retrieves the GitHub token from Atmos settings or environment. | ||
| func getGitHubToken() string { | ||
| // This would integrate with Atmos configuration system | ||
| // For now, return empty string - the actual implementation would check: | ||
| // 1. ATMOS_GITHUB_TOKEN | ||
| // 2. GITHUB_TOKEN | ||
| // 3. atmosConfig.Settings.AtmosGithubToken | ||
| return "" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Implement token lookup via Viper with ATMOS_ first.*
Meets env binding guideline and reduces rate-limit issues.
-import (
+import (
"encoding/json"
"fmt"
"io"
"net/http"
"strings"
"time"
@@
-// getGitHubToken retrieves the GitHub token from Atmos settings or environment.
+// getGitHubToken retrieves the GitHub token from environment (ATMOS_GITHUB_TOKEN first, then GITHUB_TOKEN).
func getGitHubToken() string {
- // This would integrate with Atmos configuration system
- // For now, return empty string - the actual implementation would check:
- // 1. ATMOS_GITHUB_TOKEN
- // 2. GITHUB_TOKEN
- // 3. atmosConfig.Settings.AtmosGithubToken
- return ""
+ // Prefer ATMOS_* first.
+ _ = viper.BindEnv("github_token", "ATMOS_GITHUB_TOKEN")
+ if t := strings.TrimSpace(viper.GetString("github_token")); t != "" {
+ return t
+ }
+ _ = viper.BindEnv("github_token_fallback", "GITHUB_TOKEN")
+ return strings.TrimSpace(viper.GetString("github_token_fallback"))
}And add import:
import (
@@
"time"
errUtils "github.com/cloudposse/atmos/errors"
"github.com/cloudposse/atmos/pkg/perf"
"github.com/cloudposse/atmos/pkg/schema"
+ "github.com/spf13/viper"
)Test improvements (vendor_diff_integration_test.go): - Add mockgen directive for GitOperations interface - Change expectedError string to expectedErr error sentinel - Use errors.Is() for error checking instead of string matching - Add t.Parallel() to parent test and subtests - Fix all inline comment punctuation (add periods) Performance tracking (vendor_git_interface.go): - Add perf.Track() to all wrapper methods Error handling (vendor_version_check.go): - Wrap errors with context and static errors - Fix checkGitRef to return errors instead of swallowing - Fix comment punctuation Code simplification (vendor_update.go): - Replace custom string utilities with stdlib - Remove splitString and trimString implementations - Use strings.Split() and strings.TrimSpace() Linting fixes (vendor_source_provider_github.go): - Extract defaultHTTPTimeout constant - Simplify SSH prefix trimming Documentation fixes: - Fix broken links in diff.mdx and update.mdx - Add missing punctuation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add detailed documentation for the version constraints feature in vendor manifests, including: - constraints.version field with semver syntax examples - constraints.excluded_versions with wildcard support - constraints.no_prereleases flag - Complete working example - Step-by-step explanation of how atmos vendor update processes constraints 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add comprehensive blog post announcing the vendor update and diff commands, including: - Problem statement and previous workflow challenges - Feature overview and key capabilities - Version constraints with complete examples - Multi-provider architecture explanation - Real-world usage examples - CI/CD integration guide - Migration guide for existing users 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…sibility Reframe the vendor update feature around the fundamental problem of getting left behind and the compounding cost of technical debt: - Add "The Real Problem: Compounding Technical Debt" section explaining the vicious cycle of delayed updates - Introduce AWS Shared Responsibility Model analogy for open source component maintenance - Emphasize "update friction" as the root cause keeping teams on old versions - Rewrite "Why This Matters" to focus on breaking the update debt cycle - Add key insight: "make falling behind harder" by making updates the path of least resistance This messaging shift emphasizes the strategic importance of eliminating update friction rather than just feature convenience. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
internal/exec/vendor_diff_integration_test.go (1)
11-11: Update to modern gomock import path.The import still uses the deprecated
github.com/golang/mock/gomock. Per project guidelines and learnings, this should bego.uber.org/mock/gomock.- "github.com/golang/mock/gomock" + "go.uber.org/mock/gomock"internal/exec/vendor_source_provider_github.go (2)
68-76: Escape ref names in Compare API URL.Previous review already flagged this: refs may contain slashes or special characters that need URL path escaping before insertion into the Compare API URL.
158-166: Implement token lookup via Viper with ATMOS_ first.*Previous review already flagged this: the function should use
viper.BindEnvto checkATMOS_GITHUB_TOKENfirst, then fall back toGITHUB_TOKEN, per coding guidelines.
🧹 Nitpick comments (4)
internal/exec/vendor_update_test.go (3)
9-9: Add t.Parallel() to enable concurrent test execution.Per guidelines, tests should run in parallel when possible.
func TestSplitAndTrim(t *testing.T) { + t.Parallel() + tests := []struct {
48-53: Add t.Parallel() in subtest.Subtests should also run concurrently for faster execution.
for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := splitAndTrim(tt.input, tt.sep) assert.Equal(t, tt.expected, result) })
56-56: Add period to comment.All comments must end with periods per guidelines.
-// splitString and trimString tests removed - now using standard library strings.Split and strings.TrimSpace. +// splitString and trimString tests removed - now using standard library strings.Split and strings.TrimSpace.internal/exec/vendor_update.go (1)
16-17: Consider passing atmosConfig to perf.Track.Since atmosConfig is available after initialization on line 28, you could defer a second perf.Track call or restructure to pass it for better telemetry context. Current approach is valid but suboptimal.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
internal/exec/vendor_diff_integration_test.go(1 hunks)internal/exec/vendor_git_interface.go(1 hunks)internal/exec/vendor_source_provider_github.go(1 hunks)internal/exec/vendor_update.go(1 hunks)internal/exec/vendor_update_test.go(1 hunks)internal/exec/vendor_version_check.go(1 hunks)website/blog/2025-10-21-vendor-update-and-diff.md(1 hunks)website/docs/cli/commands/vendor/diff.mdx(1 hunks)website/docs/cli/commands/vendor/update.mdx(1 hunks)website/docs/core-concepts/vendor/vendor-manifest.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use casesAlways build the website (npm run build) after documentation changes to ensure no errors, broken links, or missing assets.
Files:
website/blog/2025-10-21-vendor-update-and-diff.mdwebsite/docs/core-concepts/vendor/vendor-manifest.mdxwebsite/docs/cli/commands/vendor/update.mdxwebsite/docs/cli/commands/vendor/diff.mdx
website/**/{*.md,*.mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use definition lists
for arguments and flags in docs instead of tables.
Files:
website/blog/2025-10-21-vendor-update-and-diff.mdwebsite/docs/core-concepts/vendor/vendor-manifest.mdxwebsite/docs/cli/commands/vendor/update.mdxwebsite/docs/cli/commands/vendor/diff.mdx
website/blog/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
website/blog/*.md: PRs labeled minor or major must include a blog post in website/blog/YYYY-MM-DD-feature-name.md with required frontmatter, tags, and truncate marker.
Use appropriate audience tags (feature/enhancement/bugfix or contributors) and relevant secondary tags in blog posts.
Files:
website/blog/2025-10-21-vendor-update-and-diff.md
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Unit tests should focus on pure functions and favor table-driven tests.
Test behavior (inputs/outputs), not implementation detail; avoid tautologies and stub-only tests; remove always-skipped tests.
Always call production code within tests rather than duplicating logic under test.
Use t.Skipf with a clear reason when skipping tests; never call t.Skipf without a reason.
Use go.uber.org/mock/mockgen for interface mocks; never hand-write large mock implementations.
Add //go:generate mockgen directives for mocks in test files.
Files:
internal/exec/vendor_diff_integration_test.gointernal/exec/vendor_update_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods. Applies to single-line, multi-line, inline, and documentation comments.
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines and sort alphabetically within each group; preserve existing aliases.
Add defer perf.Track(...) in all public and critical private functions; include a blank line after the perf.Track call; use "packagename.FunctionName" as the name; pass atmosConfig when available, otherwise nil.
Wrap all returned errors using static errors (from errors/errors.go); use errors.Join to combine errors; use fmt.Errorf with %w to add context; use errors.Is for checks; never rely on err.Error() string matching.
Always use viper.BindEnv with ATMOS_ alternatives; prefer ATMOS_* first with fallback to external env vars when needed.
Prefer many small, focused files; never use //revive:disable:file-length-limit to bypass file length limits.
Use filepath.Join, os.PathSeparator, and runtime.GOOS for portable path/OS handling; use build constraints when necessary.
Files:
internal/exec/vendor_diff_integration_test.gointernal/exec/vendor_update.gointernal/exec/vendor_source_provider_github.gointernal/exec/vendor_git_interface.gointernal/exec/vendor_update_test.gointernal/exec/vendor_version_check.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Use Viper for configuration management and bind environment variables consistently.
Distinguish structured logging from UI output: UI prompts/status/errors to stderr; data/results to stdout; do not use logging for UI elements.
Prefer SDKs over invoking external binaries for cross-platform compatibility (e.g., terraform-exec instead of exec.Command("terraform")).
Files:
internal/exec/vendor_diff_integration_test.gointernal/exec/vendor_update.gointernal/exec/vendor_source_provider_github.gointernal/exec/vendor_git_interface.gointernal/exec/vendor_update_test.gointernal/exec/vendor_version_check.go
{cmd,internal}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal}/**/*.go: Most text UI must go to stderr (e.g., utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr, ...)); only data/results go to stdout.
Telemetry for commands is automatic via RootCmd.ExecuteC(); for non-standard paths, call telemetry.CaptureCmd/CaptureCmdString explicitly.
Files:
internal/exec/vendor_diff_integration_test.gointernal/exec/vendor_update.gointernal/exec/vendor_source_provider_github.gointernal/exec/vendor_git_interface.gointernal/exec/vendor_update_test.gointernal/exec/vendor_version_check.go
{pkg,internal,cmd}/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should mirror implementation naming (e.g., foo.go has foo_test.go) and be co-located with implementations.
Files:
internal/exec/vendor_diff_integration_test.gointernal/exec/vendor_update_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/vendor_update.gointernal/exec/vendor_source_provider_github.gointernal/exec/vendor_git_interface.gointernal/exec/vendor_version_check.go
**/*interface.go
📄 CodeRabbit inference engine (CLAUDE.md)
For internal interfaces, generate mocks with mockgen using -source on the interface file.
Files:
internal/exec/vendor_git_interface.go
website/docs/cli/commands/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
All new commands/flags/parameters must have Docusaurus documentation under website/docs/cli/commands//.mdx using the prescribed frontmatter and sections.
Files:
website/docs/cli/commands/vendor/update.mdxwebsite/docs/cli/commands/vendor/diff.mdx
🧠 Learnings (13)
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to pkg/git/interface.go : Use mockgen for pkg/git/interface.go to generate mocks (as exemplified).
Applied to files:
internal/exec/vendor_diff_integration_test.gointernal/exec/vendor_git_interface.go
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to **/*_test.go : Add //go:generate mockgen directives for mocks in test files.
Applied to files:
internal/exec/vendor_diff_integration_test.go
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to **/*_test.go : Use go.uber.org/mock/mockgen for interface mocks; never hand-write large mock implementations.
Applied to files:
internal/exec/vendor_diff_integration_test.go
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to pkg/filesystem/interface.go : Use mockgen for pkg/filesystem/interface.go to generate mocks (as exemplified).
Applied to files:
internal/exec/vendor_diff_integration_test.go
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to pkg/pro/interface.go : Use mockgen for pkg/pro/interface.go to generate mocks (as exemplified).
Applied to files:
internal/exec/vendor_diff_integration_test.go
📚 Learning: 2025-10-21T15:50:22.360Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.360Z
Learning: Applies to **/*interface.go : For internal interfaces, generate mocks with mockgen using -source on the interface file.
Applied to files:
internal/exec/vendor_diff_integration_test.go
📚 Learning: 2025-10-21T15:50:22.359Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.359Z
Learning: Applies to **/*.go : Always use viper.BindEnv with ATMOS_ alternatives; prefer ATMOS_* first with fallback to external env vars when needed.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
PR: cloudposse/atmos#1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
PR: cloudposse/atmos#1202
File: pkg/utils/yaml_func_exec.go:104-104
Timestamp: 2025-04-23T15:02:50.246Z
Learning: In the Atmos codebase, direct calls to `os.Getenv` should be avoided. Instead, use `viper.BindEnv` for environment variable access. This provides a consistent approach to configuration management across the codebase.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
PR: cloudposse/atmos#1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
PR: cloudposse/atmos#740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-10-11T19:06:16.131Z
Learnt from: osterman
PR: cloudposse/atmos#1599
File: pkg/ui/markdown/renderer.go:247-259
Timestamp: 2025-10-11T19:06:16.131Z
Learning: Performance tracking with `defer perf.Track()` should be reserved for functions that perform actual computational work, I/O operations, or have measurable performance impact. Simple wrapper methods that immediately delegate to other functions do not require performance tracking, as it adds unnecessary overhead without providing meaningful insights.
Applied to files:
internal/exec/vendor_git_interface.go
📚 Learning: 2025-10-21T15:50:22.359Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.359Z
Learning: Applies to **/*.go : Add defer perf.Track(...) in all public and critical private functions; include a blank line after the perf.Track call; use "packagename.FunctionName" as the name; pass atmosConfig when available, otherwise nil.
Applied to files:
internal/exec/vendor_git_interface.go
🧬 Code graph analysis (5)
internal/exec/vendor_diff_integration_test.go (4)
internal/exec/vendor_diff.go (1)
VendorDiffFlags(50-58)internal/exec/mock_vendor_git_interface.go (2)
MockGitOperations(15-18)NewMockGitOperations(26-30)pkg/schema/schema.go (3)
Context(381-396)AtmosConfiguration(27-65)Vendor(756-761)errors/errors.go (3)
ErrComponentNotFound(388-388)ErrUnsupportedVendorSource(394-394)ErrNoTagsFound(403-403)
internal/exec/vendor_update.go (7)
pkg/perf/perf.go (1)
Track(121-138)internal/exec/cli_utils.go (1)
ProcessCommandLineArgs(76-161)pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (2)
AtmosConfiguration(27-65)Vendor(756-761)pkg/config/const.go (1)
AtmosVendorConfigFileName(53-53)internal/exec/vendor_utils.go (1)
ReadAndProcessVendorConfigFile(76-106)errors/errors.go (2)
ErrVendorConfigNotFound(390-390)ErrNotImplemented(404-404)
internal/exec/vendor_source_provider_github.go (4)
internal/exec/vendor_source_provider.go (6)
VendorSourceProvider(11-24)SourceOperation(27-27)OperationListVersions(31-31)OperationVerifyVersion(34-34)OperationGetDiff(37-37)OperationFetchSource(40-40)pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (2)
ErrGitDiffFailed(391-391)ErrParseURL(13-13)
internal/exec/vendor_git_interface.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)
internal/exec/vendor_version_check.go (1)
errors/errors.go (1)
ErrGitLsRemoteFailed(395-395)
🪛 LanguageTool
website/blog/2025-10-21-vendor-update-and-diff.md
[typographical] ~21-~21: Consider using typographic quotation marks here.
Context: ... 4. Fear increases → Updates become "migration projects" instead of routine maintenance 5. **You...
(EN_QUOTES)
[grammar] ~36-~36: Use a hyphen to join words.
Context: ...n it. The same principle applies to open source infrastructure components: - **U...
(QB_NEW_EN_HYPHEN)
[uncategorized] ~41-~41: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ches, fixes, and improvements that make open source valuable. **The harder it is to update...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[typographical] ~260-~260: Consider using typographic quotation marks here.
Context: ...urces**: Basic operations, diff returns "not implemented" - Other sources (OCI, local, HTTP):...
(EN_QUOTES)
[uncategorized] ~356-~356: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...oid the painful migrations. Preserve Open Source Value: Access the continuous stream o...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
website/docs/cli/commands/vendor/update.mdx
[style] ~116-~116: Consider using the typographical ellipsis character here instead.
Context: ...s (github.com/org/repo) - ✅ Git URLs (git::https://..., git@...) - ✅ HTTPS Git URLs - ❌ OCI...
(ELLIPSIS)
[style] ~116-~116: Consider using the typographical ellipsis character here instead.
Context: ...epo) - ✅ Git URLs (git::https://..., git@...`) - ✅ HTTPS Git URLs - ❌ OCI images (no...
(ELLIPSIS)
website/docs/cli/commands/vendor/diff.mdx
[style] ~186-~186: Consider using the typographical ellipsis character here instead.
Context: ...s (github.com/org/repo) - ✅ Git URLs (git::https://..., git@...) - ✅ HTTPS Git URLs - ❌ OCI...
(ELLIPSIS)
[style] ~186-~186: Consider using the typographical ellipsis character here instead.
Context: ...epo) - ✅ Git URLs (git::https://..., git@...`) - ✅ HTTPS Git URLs - ❌ OCI images (no...
(ELLIPSIS)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build (windows)
- GitHub Check: Review Dependency Licenses
- GitHub Check: Run pre-commit hooks
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: autofix
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (13)
internal/exec/vendor_update.go (2)
102-148: Placeholder implementation acknowledged.The TODO comments and ErrNotImplemented return are consistent with the PR objectives stating this is foundational work for future vendor update logic.
162-177: Clean implementation using standard library.Good use of strings.Split and strings.TrimSpace. Addresses previous feedback correctly.
internal/exec/vendor_version_check.go (3)
16-24: Well-structured result type.Clear field names and appropriate types. UpdateType as string provides flexibility though a const-based approach could add type safety.
26-56: Solid Git tag fetching implementation.Proper timeout, error wrapping, and output parsing. Pre-allocation of slice with capacity is a nice optimization.
113-118: SHA validation assumes existence based on format.Lines 115-117 note that full commit validation requires cloning. This means the function returns true for any well-formed SHA even if it doesn't exist in the repo. Verify this behavior aligns with your use case and won't cause issues downstream.
website/docs/cli/commands/vendor/diff.mdx (1)
1-273: Comprehensive diff command documentation.Well-structured with clear examples, flag descriptions, and troubleshooting guidance. The documentation aligns nicely with the implementation.
website/blog/2025-10-21-vendor-update-and-diff.md (1)
1-410: Excellent narrative explaining the new commands.The blog post effectively communicates the problem (update debt), solution (automated commands), and practical value. Strong examples throughout.
internal/exec/vendor_diff_integration_test.go (1)
19-238: Well-structured integration tests.Good coverage of success and error scenarios. Proper use of gomock, t.Parallel(), and errors.Is(). The tests validate the vendor diff workflow comprehensively.
website/docs/core-concepts/vendor/vendor-manifest.mdx (1)
180-259: Clear constraints documentation.The new constraints section provides comprehensive examples and clear explanations. The complete example at lines 233-255 effectively demonstrates how the constraints work together.
website/docs/cli/commands/vendor/update.mdx (1)
1-180: Thorough update command documentation.Comprehensive coverage of flags, workflow, and examples. The "How It Works" section (lines 84-95) clearly explains the preservation of YAML structure, which is a key feature.
internal/exec/vendor_git_interface.go (3)
1-21: Clean interface definition with proper mock generation.The interface is well-documented and the
go:generatedirective for mockgen is correctly placed. Nice work.
31-43: Proper performance tracking in place.Both
GetRemoteTagsandCheckRefnow include the requireddefer perf.Track(...)calls with correct formatting.
45-52: Performance tracking correctly implemented.
GetDiffBetweenRefsproperly includesdefer perf.Track(...)withatmosConfigpassed through. The implementation follows guidelines.
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Resolved conflict in errors/errors.go by combining both logout-related errors (from main) and vendor update/diff errors (from feature branch). Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pkg/schema/schema.go (2)
783-792: Consider adding inline documentation for the Constraints field.The new
Constraintsfield is properly implemented with appropriate tags and pointer type for optionality. However, an inline comment would help users understand its purpose in the vendor.yaml schema.Consider adding a comment:
type AtmosVendorSource struct { Component string `yaml:"component" json:"component" mapstructure:"component"` Source string `yaml:"source" json:"source" mapstructure:"source"` Version string `yaml:"version" json:"version" mapstructure:"version"` + // Constraints specifies version constraints for vendor updates (e.g., semver ranges, excluded versions) Constraints *VendorConstraints `yaml:"constraints,omitempty" json:"constraints,omitempty" mapstructure:"constraints"` File string `yaml:"file" json:"file" mapstructure:"file"` Targets []string `yaml:"targets" json:"targets" mapstructure:"targets"` IncludedPaths []string `yaml:"included_paths,omitempty" json:"included_paths,omitempty" mapstructure:"included_paths"` ExcludedPaths []string `yaml:"excluded_paths,omitempty" json:"excluded_paths,omitempty" mapstructure:"excluded_paths"` Tags []string `yaml:"tags" json:"tags" mapstructure:"tags"` }
794-799: Consider documenting individual fields for clarity.The struct has a good doc comment, and the implementation is solid. Adding inline comments for the fields would help clarify their purpose, especially since
Versionhere represents a constraint pattern (e.g.,>=1.0.0 <2.0.0) rather than a pinned version like the parent struct'sVersionfield.Example:
// VendorConstraints defines version constraints for vendor updates. type VendorConstraints struct { + // Version specifies a semver constraint pattern (e.g., ">=1.0.0 <2.0.0") Version string `yaml:"version,omitempty" json:"version,omitempty" mapstructure:"version"` + // ExcludedVersions lists specific versions to exclude from updates ExcludedVersions []string `yaml:"excluded_versions,omitempty" json:"excluded_versions,omitempty" mapstructure:"excluded_versions"` + // NoPrereleases excludes prerelease versions from consideration NoPrereleases bool `yaml:"no_prereleases,omitempty" json:"no_prereleases,omitempty" mapstructure:"no_prereleases"` }go.mod (1)
15-15: Clarify the dual mocking libraries.The changes add
github.com/golang/mock v1.1.1(line 49), but the project already hasgo.uber.org/mock v0.6.0(line 96) as a direct dependency. These are related—Uber's version is a fork of the upstream Google library. Confirm whether both are intentional or if one can be consolidated.If the PR uses only one mocking pattern, consolidate to a single library to reduce supply-chain complexity.
Also applies to: 49-49
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
errors/errors.go(1 hunks)go.mod(2 hunks)pkg/schema/schema.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- errors/errors.go
🧰 Additional context used
📓 Path-based instructions (4)
go.{mod,sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
go.{mod,sum}: Manage dependencies with Go modules
Keep dependencies up to date
Files:
go.mod
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/schema/schema.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
Files:
pkg/schema/schema.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/schema/schema.go
🧠 Learnings (8)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to go.{mod,sum} : Keep dependencies up to date
Applied to files:
go.mod
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to go.{mod,sum} : Manage dependencies with Go modules
Applied to files:
go.mod
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
PR: cloudposse/atmos#1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
go.modpkg/schema/schema.go
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
PR: cloudposse/atmos#768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: Go version 1.23.0 was deliberately introduced by the maintainer (aknysh) in January 2025. While this might be a pre-release or development version of Go, it has been approved for use in this project.
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Applied to files:
go.mod
📚 Learning: 2024-10-31T01:22:09.586Z
Learnt from: osterman
PR: cloudposse/atmos#737
File: examples/demo-vendoring/vendor.d/vendor1.yaml:10-11
Timestamp: 2024-10-31T01:22:09.586Z
Learning: In `examples/demo-vendoring/vendor.d/vendor1.yaml`, when specifying the source for the `ipinfo` component, it's acceptable to use `main` as the version reference if pinning to a specific version is not practical.
Applied to files:
pkg/schema/schema.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Check for changelog entry
- GitHub Check: Build (linux)
- GitHub Check: Build (macos)
- GitHub Check: Build (windows)
- GitHub Check: autofix
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: website-deploy-preview
- GitHub Check: Review Dependency Licenses
- GitHub Check: Summary
🔇 Additional comments (2)
go.mod (2)
15-15: v3.4.0 is current—dependency update is sound.Verification confirms v3.4.0 (released 27 Jun 2025) is the latest v3 release, making this a safe, up-to-date dependency promotion that aligns with your feature needs.
3-3: Go 1.25.0 upgrade is intentional and properly tracked.Git history confirms this upgrade was intentional (PR #1706: "Upgrade to Go 1.25 and make test logging respect -v flag"). The version is legitimate, released August 12, 2025, and maintains backward compatibility with Go 1.23.0. No further action needed.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
NOTICE(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint (golangci)
- GitHub Check: Build (windows)
- GitHub Check: Summary
NOTICE
Outdated
| - github.com/golang/mock/gomock | ||
| License: Apache-2.0 | ||
| URL: https://github.com/golang/mock/blob/v1.1.1/LICENSE | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Replace deprecated github.com/golang/mock with go.uber.org/mock.
The github.com/golang/mock project is officially archived and deprecated. Version v1.1.1 dates from 2018. Your NOTICE file already includes the maintained fork go.uber.org/mock/gomock (Line 468-470) at v0.6.0.
Standardize on go.uber.org/mock throughout the codebase and remove this deprecated dependency to avoid maintenance risks.
Run this script to verify usage and guide migration:
🏁 Script executed:
#!/bin/bash
# Check for imports of the deprecated gomock package
echo "=== Checking for deprecated github.com/golang/mock imports ==="
rg -n --type=go 'github\.com/golang/mock' -g '!vendor/' -g '!go.mod' -g '!go.sum'
echo ""
echo "=== Checking for go.uber.org/mock imports ==="
rg -n --type=go 'go\.uber\.org/mock' -g '!vendor/' -g '!go.mod' -g '!go.sum'
echo ""
echo "=== Checking go.mod for both dependencies ==="
grep -E '(github\.com/golang/mock|go\.uber\.org/mock)' go.modLength of output: 7411
Update imports from deprecated github.com/golang/mock to go.uber.org/mock.
The codebase standardizes on go.uber.org/mock/gomock (40+ files across the project), but this PR introduces the archived package in two files. Update:
internal/exec/mock_vendor_git_interface.go:11— changegithub.com/golang/mock/gomocktogo.uber.org/mock/gomockinternal/exec/vendor_diff_integration_test.go:11— changegithub.com/golang/mock/gomocktogo.uber.org/mock/gomock
Also remove github.com/golang/mock v1.1.1 from go.mod.
🤖 Prompt for AI Agents
In NOTICE around lines 272-275 and the referenced files, the project uses the
deprecated import path github.com/golang/mock/gomock; update the two mentions to
go.uber.org/mock/gomock: change the import in
internal/exec/mock_vendor_git_interface.go line ~11 and in
internal/exec/vendor_diff_integration_test.go line ~11 to use
go.uber.org/mock/gomock, run go mod tidy and remove the module entry
github.com/golang/mock v1.1.1 from go.mod (or replace it with the correct
go.uber.org/mock dependency) so the module graph is consistent.
- Replace deprecated github.com/golang/mock with go.uber.org/mock in
mock_vendor_git_interface.go and vendor_diff_integration_test.go
- Add performance tracking to vendor source provider functions:
- CheckGitHubRateLimit with error wrapping
- resolveVersionConstraints
- GetProviderForSource
- UnsupportedSourceProvider methods (GetAvailableVersions,
VerifyVersion, GetDiff)
- Fix documentation link in vendor update.mdx to point to correct
/core-concepts/vendor path
- Run go mod tidy to update dependencies and remove deprecated module
All changes follow CLAUDE.md performance tracking guidelines with
defer perf.Track() and proper error wrapping with context.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
internal/exec/vendor_source_provider_github.go (2)
70-72: Escape ref names before hitting the Compare API.Branch/tag names routinely contain slashes; leaving them raw produces malformed URLs (404 from GitHub). Please path-escape both refs when building
compareURLand add thenet/urlimport.+ "net/url" @@ - compareURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/compare/%s...%s", - owner, repo, fromVersion, toVersion) + compareURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/compare/%s...%s", + owner, repo, url.PathEscape(fromVersion), url.PathEscape(toVersion))
158-166: Implement GitHub token lookup.Shipping with
getGitHubTokenreturning""means no auth—private repos fail and rate limits hit immediately. Please wire up env/config lookups (ATMOS_GITHUB_TOKEN first, then GITHUB_TOKEN, then any configured value).-import ( +import ( @@ - "time" + "time" @@ - "strings" + "strings" @@ - "github.com/cloudposse/atmos/pkg/schema" + "github.com/cloudposse/atmos/pkg/schema" + "github.com/spf13/viper" @@ -func getGitHubToken() string { - // This would integrate with Atmos configuration system - // For now, return empty string - the actual implementation would check: - // 1. ATMOS_GITHUB_TOKEN - // 2. GITHUB_TOKEN - // 3. atmosConfig.Settings.AtmosGithubToken - return "" +func getGitHubToken() string { + if err := viper.BindEnv("github_token", "ATMOS_GITHUB_TOKEN"); err == nil { + if token := strings.TrimSpace(viper.GetString("github_token")); token != "" { + return token + } + } + + if err := viper.BindEnv("github_token_fallback", "GITHUB_TOKEN"); err == nil { + if token := strings.TrimSpace(viper.GetString("github_token_fallback")); token != "" { + return token + } + } + + return "" }
🧹 Nitpick comments (1)
website/docs/cli/commands/vendor/update.mdx (1)
9-11: Trim unused Terminal import.
Terminalnever gets referenced, so keeping the import only bloats the bundle. Please drop it.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
go.mod(1 hunks)internal/exec/mock_vendor_git_interface.go(1 hunks)internal/exec/vendor_diff_integration_test.go(1 hunks)internal/exec/vendor_source_provider.go(1 hunks)internal/exec/vendor_source_provider_github.go(1 hunks)internal/exec/vendor_source_provider_unsupported.go(1 hunks)internal/exec/vendor_version_constraints.go(1 hunks)website/docs/cli/commands/vendor/update.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/exec/vendor_diff_integration_test.go
- go.mod
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
Files:
internal/exec/vendor_version_constraints.gointernal/exec/mock_vendor_git_interface.gointernal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_source_provider_github.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/vendor_version_constraints.gointernal/exec/mock_vendor_git_interface.gointernal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_source_provider_github.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/docs/cli/commands/vendor/update.mdx
🧠 Learnings (34)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Learnt from: osterman
PR: cloudposse/atmos#1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
📚 Learning: 2024-10-22T23:00:20.627Z
Learnt from: Cerebrovinny
PR: cloudposse/atmos#737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Applied to files:
internal/exec/vendor_version_constraints.gointernal/exec/vendor_source_provider.go
📚 Learning: 2025-10-11T19:12:23.475Z
Learnt from: osterman
PR: cloudposse/atmos#1599
File: pkg/ui/markdown/renderer.go:143-182
Timestamp: 2025-10-11T19:12:23.475Z
Learning: High-frequency utility and formatting functions (e.g., markdown renderers, formatters) that are called repeatedly during command execution should not have `defer perf.Track()` even if they are public methods. Performance tracking at these levels introduces unnecessary overhead without meaningful insights, as tracking is already present at higher command-execution levels where it provides actionable data.
Applied to files:
internal/exec/vendor_version_constraints.gointernal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_source_provider_github.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
PR: cloudposse/atmos#1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
internal/exec/vendor_version_constraints.gointernal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_source_provider_github.go
📚 Learning: 2025-10-11T19:06:16.131Z
Learnt from: osterman
PR: cloudposse/atmos#1599
File: pkg/ui/markdown/renderer.go:247-259
Timestamp: 2025-10-11T19:06:16.131Z
Learning: Performance tracking with `defer perf.Track()` should be reserved for functions that perform actual computational work, I/O operations, or have measurable performance impact. Simple wrapper methods that immediately delegate to other functions do not require performance tracking, as it adds unnecessary overhead without providing meaningful insights.
Applied to files:
internal/exec/vendor_version_constraints.gointernal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_source_provider_github.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
PR: cloudposse/atmos#808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
internal/exec/vendor_version_constraints.gointernal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_source_provider_github.go
📚 Learning: 2024-10-31T07:09:31.983Z
Learnt from: Cerebrovinny
PR: cloudposse/atmos#737
File: internal/exec/vendor_utils.go:181-182
Timestamp: 2024-10-31T07:09:31.983Z
Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.
Applied to files:
internal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_unsupported.go
📚 Learning: 2025-01-25T03:59:47.777Z
Learnt from: Listener430
PR: cloudposse/atmos#934
File: internal/exec/docs_generate.go:220-248
Timestamp: 2025-01-25T03:59:47.777Z
Learning: In the `downloadSource` function in `internal/exec/docs_generate.go`, the code is intentionally kept concise without additional security measures like content validation, download size limits, or domain validation to maintain simplicity and focus on core functionality.
Applied to files:
internal/exec/vendor_source_provider.gointernal/exec/vendor_source_provider_github.go
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
PR: cloudposse/atmos#768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
internal/exec/vendor_source_provider_unsupported.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
PR: cloudposse/atmos#740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
internal/exec/vendor_source_provider_unsupported.gointernal/exec/vendor_source_provider_github.go
📚 Learning: 2025-09-10T21:17:55.273Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Applied to files:
internal/exec/vendor_source_provider_unsupported.go
📚 Learning: 2025-02-05T11:10:51.031Z
Learnt from: mss
PR: cloudposse/atmos#1024
File: internal/exec/go_getter_utils.go:31-33
Timestamp: 2025-02-05T11:10:51.031Z
Learning: The path traversal check in `ValidateURI` function in `internal/exec/go_getter_utils.go` is intentionally kept despite potentially blocking valid Git URLs, as this validation is planned to be addressed in a separate ticket.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-02-03T15:51:48.035Z
Learnt from: osterman
PR: cloudposse/atmos#984
File: internal/exec/go_getter_utils.go:103-109
Timestamp: 2025-02-03T15:51:48.035Z
Learning: When checking for subdirectories in GitHub URLs, use `parsedURL.Path` to check for "//" instead of the entire URL, as the scheme portion (e.g., "https://") will always contain "//".
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2024-10-23T20:13:23.054Z
Learnt from: osterman
PR: cloudposse/atmos#731
File: pkg/utils/file_utils.go:198-202
Timestamp: 2024-10-23T20:13:23.054Z
Learning: In `pkg/utils/file_utils.go`, the current implementation of the `IsURL` function is considered sufficient; avoid suggesting more complex URL validation in future reviews.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-02-04T22:45:15.845Z
Learnt from: osterman
PR: cloudposse/atmos#0
File: :0-0
Timestamp: 2025-02-04T22:45:15.845Z
Learning: When validating URLs in Go, use the standard `url.Parse` function instead of character-based validation to properly handle URL-safe characters and query parameters.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
PR: cloudposse/atmos#887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
PR: cloudposse/atmos#1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
PR: cloudposse/atmos#1202
File: pkg/utils/yaml_func_exec.go:104-104
Timestamp: 2025-04-23T15:02:50.246Z
Learning: In the Atmos codebase, direct calls to `os.Getenv` should be avoided. Instead, use `viper.BindEnv` for environment variable access. This provides a consistent approach to configuration management across the codebase.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
PR: cloudposse/atmos#1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-09-10T17:34:52.568Z
Learnt from: Benbentwo
PR: cloudposse/atmos#1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2024-12-15T10:20:08.436Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/cmd_utils.go:454-464
Timestamp: 2024-12-15T10:20:08.436Z
Learning: Avoid adding timeout handling for GitHub API calls in `CheckForAtmosUpdateAndPrintMessage` function in `cmd/cmd_utils.go`, as it might be disabled by user settings.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-01-09T19:53:29.847Z
Learnt from: Listener430
PR: cloudposse/atmos#912
File: pkg/config/config.go:91-92
Timestamp: 2025-01-09T19:53:29.847Z
Learning: In the Atmos project, the `core.inject_github_token` configuration is required to be enabled (`true`) by default to support authenticated GitHub requests and help bypass rate limits.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-01-09T20:02:37.990Z
Learnt from: osterman
PR: cloudposse/atmos#912
File: website/docs/cli/configuration/configuration.mdx:113-114
Timestamp: 2025-01-09T20:02:37.990Z
Learning: The `inject_github_token` setting should be placed under the top-level `settings` section in `atmos.yaml` instead of creating a new `core` section.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
PR: cloudposse/atmos#1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*.go : Wrap errors with context using fmt.Errorf("context: %w", err)
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2024-12-05T02:48:53.818Z
Learnt from: aknysh
PR: cloudposse/atmos#809
File: cmd/cmd_utils.go:470-477
Timestamp: 2024-12-05T02:48:53.818Z
Learning: The function `GetLatestGitHubRepoRelease` in the Go codebase already uses a context with a timeout, so wrapping it with an additional context is unnecessary.
Applied to files:
internal/exec/vendor_source_provider_github.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to README.md : Update README.md when adding new commands and features
Applied to files:
website/docs/cli/commands/vendor/update.mdx
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
website/docs/cli/commands/vendor/update.mdx
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to website/** : Keep CLI and website documentation in sync; document new features with examples and use cases
Applied to files:
website/docs/cli/commands/vendor/update.mdx
📚 Learning: 2024-11-12T13:06:56.194Z
Learnt from: osterman
PR: cloudposse/atmos#768
File: website/docs/cheatsheets/vendoring.mdx:70-70
Timestamp: 2024-11-12T13:06:56.194Z
Learning: In `atmos vendor pull --everything`, the `--everything` flag uses the TTY for TUI but is not interactive.
Applied to files:
website/docs/cli/commands/vendor/update.mdx
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
PR: cloudposse/atmos#1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
website/docs/cli/commands/vendor/update.mdx
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Applied to files:
website/docs/cli/commands/vendor/update.mdx
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
website/docs/cli/commands/vendor/update.mdx
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
website/docs/cli/commands/vendor/update.mdx
🧬 Code graph analysis (5)
internal/exec/vendor_version_constraints.go (3)
pkg/schema/schema.go (1)
VendorConstraints(795-799)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (3)
ErrNoVersionsAvailable(453-453)ErrNoVersionsMatchConstraints(454-454)ErrInvalidSemverConstraint(455-455)
internal/exec/mock_vendor_git_interface.go (1)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)
internal/exec/vendor_source_provider.go (5)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)internal/exec/vendor_source_provider_github.go (1)
NewGitHubSourceProvider(27-33)internal/exec/vendor_source_provider_git.go (1)
NewGenericGitSourceProvider(16-18)internal/exec/vendor_source_provider_unsupported.go (1)
NewUnsupportedSourceProvider(16-18)
internal/exec/vendor_source_provider_unsupported.go (4)
internal/exec/vendor_source_provider.go (2)
VendorSourceProvider(12-25)SourceOperation(28-28)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (1)
ErrUnsupportedVendorSource(443-443)pkg/schema/schema.go (1)
AtmosConfiguration(27-65)
internal/exec/vendor_source_provider_github.go (4)
internal/exec/vendor_source_provider.go (6)
VendorSourceProvider(12-25)SourceOperation(28-28)OperationListVersions(32-32)OperationVerifyVersion(35-35)OperationGetDiff(38-38)OperationFetchSource(41-41)pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (5)
ErrGitDiffFailed(440-440)ErrParseURL(13-13)ErrFailedToCreateRequest(258-258)ErrHTTPRequestFailed(118-118)ErrFailedToUnmarshalAPIResponse(277-277)
🪛 LanguageTool
website/docs/cli/commands/vendor/update.mdx
[style] ~116-~116: Consider using the typographical ellipsis character here instead.
Context: ...s (github.com/org/repo) - ✅ Git URLs (git::https://..., git@...) - ✅ HTTPS Git URLs - ❌ OCI...
(ELLIPSIS)
[style] ~116-~116: Consider using the typographical ellipsis character here instead.
Context: ...epo) - ✅ Git URLs (git::https://..., git@...`) - ✅ HTTPS Git URLs - ❌ OCI images (no...
(ELLIPSIS)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: autofix
- GitHub Check: Summary
🔇 Additional comments (2)
internal/exec/vendor_source_provider.go (2)
10-25: Clean interface design.The VendorSourceProvider interface is well-structured with clear method signatures and documentation. The strategy pattern enables pluggable implementations for different source types.
44-61: Verification passed—helper functions exist and code is sound.Both
isGitHubSourceandisGitSourceare properly defined in their respective provider files. The function implements a clear fallback chain with proper instrumentation and documentation.
| return nil, fmt.Errorf("%w: failed to create request: %s", errUtils.ErrGitDiffFailed, err) | ||
| } | ||
|
|
||
| // Add GitHub API headers | ||
| req.Header.Set("Accept", "application/vnd.github.v3.diff") | ||
|
|
||
| // Add authentication if available (from environment) | ||
| if token := getGitHubToken(); token != "" { | ||
| req.Header.Set("Authorization", "Bearer "+token) | ||
| } | ||
|
|
||
| resp, err := g.httpClient.Do(req) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("%w: failed to fetch diff from GitHub: %s", errUtils.ErrGitDiffFailed, err) | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| body, _ := io.ReadAll(resp.Body) | ||
| return nil, fmt.Errorf("%w: GitHub API returned status %d: %s", | ||
| errUtils.ErrGitDiffFailed, resp.StatusCode, string(body)) | ||
| } | ||
|
|
||
| diff, err := io.ReadAll(resp.Body) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("%w: failed to read diff: %s", errUtils.ErrGitDiffFailed, err) |
There was a problem hiding this comment.
Preserve original errors with %w.
Right now we stringify the underlying error, so callers can’t unwrap it. Swap each %s for %w so the wrapped errors remain discoverable (errors.Is/As).
- return nil, fmt.Errorf("%w: failed to create request: %s", errUtils.ErrGitDiffFailed, err)
+ return nil, fmt.Errorf("%w: failed to create request: %w", errUtils.ErrGitDiffFailed, err)
@@
- return nil, fmt.Errorf("%w: failed to fetch diff from GitHub: %s", errUtils.ErrGitDiffFailed, err)
+ return nil, fmt.Errorf("%w: failed to fetch diff from GitHub: %w", errUtils.ErrGitDiffFailed, err)
@@
- return nil, fmt.Errorf("%w: failed to read diff: %s", errUtils.ErrGitDiffFailed, err)
+ return nil, fmt.Errorf("%w: failed to read diff: %w", errUtils.ErrGitDiffFailed, err)🤖 Prompt for AI Agents
internal/exec/vendor_source_provider_github.go lines 75-100: the fmt.Errorf
calls currently use "%w" for errUtils.ErrGitDiffFailed and "%s" for the
underlying err, which prevents unwrapping the real error; change these calls so
the underlying err is wrapped with "%w" and include errUtils.ErrGitDiffFailed
with "%v" (i.e., swap the formatting to wrap the actual error with %w and render
the sentinel error as %v), leaving the GitHub status/body error formatting
unchanged.
Resolved conflict in errors/errors.go by combining: - ErrKeychainDeletionRequiresConfirmation from main - Vendor update/diff errors from current branch Also fixed lint issues: - Changed %s to %w for proper error wrapping (errorlint) - Used errors.As instead of type assertion for exec.ExitError - Added defer perf.Track() calls to public functions (lintroller)
…ing/ This completes the vendor command refactoring: Phase 1: Move vendor commands from flat cmd/ to cmd/vendor/ directory - Created cmd/vendor/ with CommandProvider pattern - Moved vendor.go, diff.go, pull.go, update.go, utils.go - Updated cmd/root.go with blank import Phase 2: Move vendor business logic from internal/exec/ to pkg/vendoring/ - Created pkg/vendoring/ package with all vendor functionality - Preserved all existing code structure and tests - Updated imports in cmd/vendor/ to use new package Phase 3: Create uri subpackage for URI helpers - Created pkg/vendoring/uri/ subpackage with exported functions - Renamed package from pkg/vendor to pkg/vendoring to avoid Go's special /vendor/ path handling restriction - Updated all imports to use alias for backward compatibility Linting fixes: - Fixed errorlint: %v to %w for proper error wrapping - Added perf.Track() to all public functions - Added constants for repeated flag name strings - Refactored determineSourceType to return struct (max 3 returns) - Removed unused copyToTarget function and cp import - Updated pkg/vender tests to import from pkg/vendoring 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (3)
errors/errors.go (1)
574-574: Naming inconsistency noted in prior review.
ErrVendorConfigNotFoundvs the PRD'sErrVendorConfigFileNotFound— pick one canonical name for consistency across code and docs.pkg/vendoring/source_provider_github.go (2)
74-77: Escape ref names in Compare API URL.Refs may contain slashes (e.g.,
refs/heads/feature/foo). Path-escape both version parameters to avoid malformed URLs.This was flagged in a previous review. Add
net/urlimport and useurl.PathEscape()forfromVersionandtoVersion.
166-174: Implement token lookup via Viper with ATMOS_ prefix.*The stub should use
viper.BindEnvper coding guidelines, preferringATMOS_GITHUB_TOKENfirst.This was flagged in a previous review. The implementation should check:
atmosConfig.Settings.AtmosGithubToken(if atmosConfig available)ATMOS_GITHUB_TOKENvia ViperGITHUB_TOKENas fallback
🧹 Nitpick comments (36)
docs/prd/vendor-refactoring-plan.md (3)
98-98: Add blank lines around markdown tables.Tables should be surrounded by blank lines for proper markdown rendering and linter compliance.
Apply this diff to add blank lines:
| Function | File | Action | |----------|------|--------| | `Update` | update.go | Add unit tests with mocked config | + ### Priority 2: Low-coverage functions (38-70%) + | Function | Coverage | Action | |----------|----------|--------|Also applies to: 108-108
124-124: Specify language for fenced code blocks.Code blocks should declare their language for syntax highlighting and linter compliance.
Apply this diff to specify languages:
-``` +``` pkg/vendor/And:
-``` +```mermaid pkg/vendor (main)Also applies to: 162-162
48-48: Add question mark to prompt text.Line 48 presents a question; format as "Why?" for clarity.
Apply this diff:
-**Why:** Clean separation of git operations, enables better mocking. +**Why?** Clean separation of git operations, enables better mocking.pkg/vendoring/version_constraints.go (1)
17-22: Perf track name should reflect the vendoring package.The function is in
vendoringpackage but the track name saysexec.resolveVersionConstraints.func resolveVersionConstraints( availableVersions []string, constraints *schema.VendorConstraints, ) (string, error) { - defer perf.Track(nil, "exec.resolveVersionConstraints")() + defer perf.Track(nil, "vendoring.resolveVersionConstraints")()pkg/vendoring/component_utils.go (2)
14-26: Logger import may conflict with conventions.Line 16 imports
github.com/jfrog/jfrog-client-go/utils/log, but the coding guidelines specify usinglog "github.com/cloudposse/atmos/pkg/logger"alias. Consider aligning with the project's logger for consistency.- "github.com/jfrog/jfrog-client-go/utils/log" + log "github.com/cloudposse/atmos/pkg/logger"
51-52: Perf track name uses wrong package prefix.The function is in
vendoringpackage but tracks asexec.ReadAndProcessComponentVendorConfigFile.- defer perf.Track(atmosConfig, "exec.ReadAndProcessComponentVendorConfigFile")() + defer perf.Track(atmosConfig, "vendoring.ReadAndProcessComponentVendorConfigFile")()Same applies to lines 115 and 224.
pkg/vendoring/model.go (1)
53-61: perf tracking is wired correctly; consider naming metrics undervendoring.*The added
perf.Trackcalls onpkgType.String,modelVendor.Update,modelVendor.View, andExecuteInstallfollow the repo pattern and usenil/atmosConfigappropriately.Minor naming thought: these live in the
vendoringpackage but are tracked asexec.String,exec.Update,exec.View, andexec.ExecuteInstall. If you care about perf dashboards by package, you might eventually want to rename these labels to something likevendoring.String/vendoring.ExecuteInstallto reflect the actual package boundary. No functional impact either way.Also applies to: 196-225, 303-305, 494-513
pkg/vendoring/git_diff_test.go (1)
98-160: MakeTestShouldColorizeOutputmore deterministic across environmentsRight now the “all conditions met for colorization” case assumes tests always run without a TTY and only skips on CI. If
shouldColorizeOutputis wired to a real TTY probe, that assumption can flip depending on howgo testis invoked, which makes this test brittle.If feasible, consider injecting the TTY check (or TERM/TTY state) into
shouldColorizeOutputso the test can force a known state, or broaden the assertion in this one case (e.g., only assertingfalsewhen TERM is clearly non‑color or outputFile is set, and skipping when relying on an actual TTY probe).pkg/vendoring/yaml_updater_test.go (2)
14-120: Consider explicitly asserting comment preservation in the “comments preserved” caseThe “update version with comments preserved” scenario currently verifies the version change via
findComponentVersion, which is good. Since preserving YAML comments is a key requirement, you might also want to read the updated file back andassert.Containson the relevant comments (e.g.,# Vendor configuration,# current stable version) to ensure they survive the round‑trip.That would make this test concretely enforce the preservation guarantee rather than just the version update.
122-205: Tighten not‑found assertions with sentinel error checksThe not‑found cases currently use a generic
assert.Error. IffindComponentVersion(andupdateYAMLVersion) return a well‑known vendoring error likeerrUtils.ErrComponentNotFound(or similar), it’d be more robust to assert withassert.ErrorIsagainst that sentinel. That way you’re testing the public error contract, not just “some error occurred.”cmd/vendor/update.go (1)
46-53: Consider adding examples to command help.Per coding guidelines, commands should include examples in help text for better user experience.
var updateCmd = &cobra.Command{ Use: "update", Short: "Check for and apply vendor component updates", Long: "Check for available updates to vendored components and optionally update the vendor configuration to use newer versions.", + Example: ` # Check for updates without modifying files + atmos vendor update --check + + # Update a specific component + atmos vendor update --component vpc + + # Update and pull components + atmos vendor update --pull`, FParseErrWhitelist: struct{ UnknownFlags bool }{UnknownFlags: false}, Args: cobra.NoArgs, RunE: runUpdate, }pkg/vendoring/diff.go (2)
116-133: Clarify tag selection behavior when no semver tags exist.When no semantic version tags are found, the code falls back to
tags[0]. The order of tags fromgit ls-remoteisn't guaranteed to be chronologically sorted. This could result in an arbitrary tag being selected rather than the most recent.Consider documenting this behavior or sorting tags by commit date if a predictable "latest" is needed.
151-163: TODO marker present - offering assistance.Component vendor diff for
component.yamlfiles is stubbed. The TODO outlines the implementation path clearly.Want me to help implement the component.yaml diff logic or open an issue to track this?
pkg/vendoring/version_check.go (2)
37-53: Edge case: empty output produces single empty string.When
strings.TrimSpaceon empty output returns"",strings.Split("", "\n")produces[]string{""}. The loop handles this gracefully since the parts check filters it out, but you could short-circuit earlier.lines := strings.Split(strings.TrimSpace(string(output)), "\n") + if len(lines) == 1 && lines[0] == "" { + return []string{}, nil + } tags := make([]string, 0, len(lines))
123-128: Consider case-insensitive SHA matching.Git accepts uppercase hex in SHAs. Current regex only matches lowercase
[0-9a-f].- matched, _ := regexp.MatchString(`^[0-9a-f]{7,40}$`, ref) + matched, _ := regexp.MatchString(`^[0-9a-fA-F]{7,40}$`, ref)pkg/vendoring/git_interface.go (1)
26-54: Perf tracking names use "exec." prefix but this is "vendoring" package.The track names like
"exec.NewGitOperations"don't match the actual packagevendoring. Consider using"vendoring.NewGitOperations"for consistency and accurate profiling attribution.- defer perf.Track(nil, "exec.NewGitOperations")() + defer perf.Track(nil, "vendoring.NewGitOperations")()Same applies to other methods in this file.
pkg/vendoring/yaml_updater.go (1)
15-54: updateYAMLVersion correctly orchestrates the YAML update flow.Good use of yaml.Node to preserve structure. Error handling wraps with static errors.
Same note as git_interface.go: perf track name uses
"exec.updateYAMLVersion"but should be"vendoring.updateYAMLVersion".pkg/vendoring/mock_git_interface.go (1)
4-5: Package comment mismatch in generated file.The generated comment says "Package exec" but the actual package is
vendoring. This stems from the mockgen source configuration. Consider regenerating with the correct package name in the source directive or updating the//go:generatedirective.pkg/vendoring/source_provider_unsupported.go (2)
17-17: Perf track name uses wrong package prefix.The track name uses
"exec.NewUnsupportedSourceProvider"but this file is in thevendoringpackage. Should be"vendoring.NewUnsupportedSourceProvider"for consistency.- defer perf.Track(nil, "exec.NewUnsupportedSourceProvider")() + defer perf.Track(nil, "vendoring.NewUnsupportedSourceProvider")()
24-55: Same prefix issue across all methods.All perf.Track calls use
"exec."prefix instead of"vendoring.". Consider updating them for consistency with the package name.- defer perf.Track(nil, "exec.UnsupportedSourceProvider.GetAvailableVersions")() + defer perf.Track(nil, "vendoring.UnsupportedSourceProvider.GetAvailableVersions")()Similar updates for
VerifyVersion,GetDiff, andSupportsOperation.pkg/vendoring/git_diff.go (3)
80-85: Suspicious viper.BindEnv call.
viper.BindEnv("ATMOS_TERM", "ATMOS_TERM", "TERM")has the config key and first env var as the same value. Per guidelines, this should likely be:- _ = viper.BindEnv("ATMOS_TERM", "ATMOS_TERM", "TERM") - term := viper.GetString("ATMOS_TERM") + _ = viper.BindEnv("term", "ATMOS_TERM", "TERM") + term := viper.GetString("term")Or use a simpler approach reading directly from
os.Getenv("TERM")since this is a standard env var.
112-112: Perf track uses wrong package prefix.Same issue as other files - uses
"exec.getGitDiffBetweenRefs"but should be"vendoring.getGitDiffBetweenRefs".
125-135: Git commands lack timeout.Using
context.Background()means these git commands could hang indefinitely on network issues. Consider adding a timeout context.- ctx := context.Background() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel()This applies to the entire function's context usage.
pkg/vendoring/pull.go (1)
87-87: Unusedargsparameter.The
argsparameter is alwaysnilfrom thePullcall (line 68). The check at lines 114-117 is effectively dead code.Either remove the parameter or document its intended future use.
-func handleVendorConfig(atmosConfig *schema.AtmosConfiguration, flg *vendorFlags, args []string) error { +func handleVendorConfig(atmosConfig *schema.AtmosConfiguration, flg *vendorFlags) error {And update the call site:
- return handleVendorConfig(atmosConfig, flg, nil) + return handleVendorConfig(atmosConfig, flg)cmd/vendor/pull.go (2)
52-59: Consider adding usage examples to command help.Per coding guidelines, Cobra commands should include examples in help text. Adding an
Examplefield would improve discoverability.var pullCmd = &cobra.Command{ Use: "pull", Short: "Pull the latest vendor configurations or dependencies", Long: "Pull and update vendor-specific configurations or dependencies to ensure the project has the latest required resources.", + Example: ` # Pull all vendor dependencies + atmos vendor pull --everything + + # Pull a specific component + atmos vendor pull --component vpc`, FParseErrWhitelist: struct{ UnknownFlags bool }{UnknownFlags: false}, Args: cobra.NoArgs, RunE: runPull, }
131-133: Define constants for all flag names to maintain consistency.In
parsePullOptions(lines 131-136), onlyflagComponentandflagTypeconstants are used while other flags ("stack","tags","dry-run","everything") use string literals. For consistency and maintainability, define constants for all flags incmd/vendor/utils.go:const ( flagComponent = "component" + flagStack = "stack" flagType = "type" + flagTags = "tags" + flagDryRun = "dry-run" + flagEverything = "everything" )Then update
parsePullOptionsto use these constants:return &PullOptions{ Component: v.GetString(flagComponent), - Stack: v.GetString("stack"), + Stack: v.GetString(flagStack), ComponentType: v.GetString(flagType), - Tags: v.GetString("tags"), - DryRun: v.GetBool("dry-run"), - Everything: v.GetBool("everything"), + Tags: v.GetString(flagTags), + DryRun: v.GetBool(flagDryRun), + Everything: v.GetBool(flagEverything), }, nilpkg/vendoring/source_provider_git.go (3)
23-29: Consider wrapping errors with context.When
getGitRemoteTagsfails, the error lacks context about which source was queried. This makes debugging harder.func (g *GenericGitSourceProvider) GetAvailableVersions(source string) ([]string, error) { defer perf.Track(nil, "exec.GenericGitSourceProvider.GetAvailableVersions")() gitURI := extractGitURI(source) - return getGitRemoteTags(gitURI) + tags, err := getGitRemoteTags(gitURI) + if err != nil { + return nil, fmt.Errorf("failed to get versions from %s: %w", source, err) + } + return tags, nil }
31-37: Same pattern - wrap errors with source context.func (g *GenericGitSourceProvider) VerifyVersion(source string, version string) (bool, error) { defer perf.Track(nil, "exec.GenericGitSourceProvider.VerifyVersion")() gitURI := extractGitURI(source) - return checkGitRef(gitURI, version) + exists, err := checkGitRef(gitURI, version) + if err != nil { + return false, fmt.Errorf("failed to verify version %s in %s: %w", version, source, err) + } + return exists, nil }
64-65: Missing period on comment.Per coding guidelines, all comments must end with periods.
case OperationGetDiff: - return false // Not implemented for generic Git + return false // Not implemented for generic Git.pkg/vendoring/update.go (1)
106-121: Utility function is correct but could use pre-allocation.The function works correctly. Minor optimization: pre-allocate slice capacity when input is non-empty.
func splitAndTrim(s, delimiter string) []string { if s == "" { return nil } - parts := []string{} + rawParts := strings.Split(s, delimiter) + parts := make([]string, 0, len(rawParts)) - for _, part := range strings.Split(s, delimiter) { + for _, part := range rawParts { trimmed := strings.TrimSpace(part) if trimmed != "" { parts = append(parts, trimmed) } } return parts }pkg/vendoring/source_provider_github.go (2)
26-35: Perf tracking namespace mismatch.The perf.Track calls use "exec." prefix but the package is now "vendoring". This should be updated for accurate profiling.
func NewGitHubSourceProvider() VendorSourceProvider { - defer perf.Track(nil, "exec.NewGitHubSourceProvider")() + defer perf.Track(nil, "vendoring.NewGitHubSourceProvider")() return &GitHubSourceProvider{Same applies to other functions in this file (lines 39, 48, 66, 118, 189).
109-111: Offer to implement TODO items.These TODOs indicate incomplete functionality for file filtering, context lines configuration, and color stripping.
Want me to generate implementations for these? I can provide:
- File path filtering using
strings.HasPrefixon diff hunks- Context line configuration (note: GitHub API returns full diff, client-side trimming needed)
- ANSI code stripping for
noColormodepkg/vendoring/uri/helpers.go (1)
17-22: Perf tracking on trivial predicates.Functions like
IsFileURI,IsOCIURI,HasLocalPathPrefixare simplestrings.HasPrefixcalls. The perf tracking overhead may exceed the operation time.Per retrieved learnings, the repo enforces perf tracking via linting for consistency. If that's the policy, keep as-is. Otherwise, consider removing tracking from these O(1) predicates to reduce noise in performance reports.
pkg/vendoring/utils.go (3)
15-22: Importing internal package from pkg.
pkg/vendoringimportsinternal/exec. This creates a dependency from public package to internal implementation. Consider whether these exec utilities should be moved to a shared location.This works but may limit flexibility. If vendoring is meant to be a public API, relying on internal packages restricts future refactoring options.
72-72: Perf tracking namespace mismatch.Uses "exec.ReadAndProcessVendorConfigFile" but package is "vendoring".
- defer perf.Track(atmosConfig, "exec.ReadAndProcessVendorConfigFile")() + defer perf.Track(atmosConfig, "vendoring.ReadAndProcessVendorConfigFile")()Same applies to line 193.
519-553: Pointer parameter mutation may cause confusion.
determineSourceTypetakes*stringand modifies it (lines 525, 535, 547). This side effect isn't obvious from the function signature or return type.Consider returning the normalized URI as part of the result struct instead of mutating the input:
type sourceTypeResult struct { UseOciScheme bool UseLocalFileSystem bool SourceIsLocalFile bool + NormalizedURI string } -func determineSourceType(sourceURI *string, vendorConfigFilePath string) (sourceTypeResult, error) { +func determineSourceType(sourceURI string, vendorConfigFilePath string) (sourceTypeResult, error) {This makes the function pure and easier to reason about.
| if err := vendorParser.BindToViper(viper.GetViper()); err != nil { | ||
| panic(err) | ||
| log.Error("Failed to bind vendor list flags to viper", "error", err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the full context of the code in question
cat -n cmd/list/vendor.go | head -100 | tail -30Repository: cloudposse/atmos
Length of output: 1248
🏁 Script executed:
# Search for all BindToViper usages and their error handling
rg -n -C3 'BindToViper' cmd/ --type goRepository: cloudposse/atmos
Length of output: 5625
🏁 Script executed:
# Check if there's a BindToViper definition and understand what it does
rg -n -B5 -A10 'func.*BindToViper' --type goRepository: cloudposse/atmos
Length of output: 5779
🏁 Script executed:
# Search for other flag binding error patterns in cmd/ to understand conventions
rg -n -C2 'err := .*Bind' cmd/ --type go | head -50Repository: cloudposse/atmos
Length of output: 2423
🏁 Script executed:
# Verify the context - confirm this is in init()
sed -n '60,90p' cmd/list/vendor.go | head -40Repository: cloudposse/atmos
Length of output: 1132
🏁 Script executed:
# Check what StandardParser.BindToViper does and whether errors should panic
sed -n '325,360p' pkg/flags/standard.goRepository: cloudposse/atmos
Length of output: 1429
Use panic() to align with codebase convention for BindToViper errors.
The codebase consistently uses panic(err) when BindToViper() fails in init() across 11+ commands (version.go, workflows.go, values.go, themes.go, stacks.go, settings.go, metadata.go, instances.go, components.go). Only vendor.go and root.go deviate by logging errors, silently allowing the command to initialize without environment variable support—a developer misconfiguration that should fail fast during initialization.
🤖 Prompt for AI Agents
In cmd/list/vendor.go around lines 85 to 87, the BindToViper error is currently
logged and swallowed which deviates from the project convention; replace the
log.Error call with a panic by calling panic(err) (or panic(fmt.Errorf("bind
vendor list flags to viper: %w", err)) if you want context) so initialization
fails fast when BindToViper returns an error.
| func TestWriteOutput(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| data []byte | ||
| outputFile string | ||
| expectErr bool | ||
| }{ | ||
| { | ||
| name: "write to file", | ||
| data: []byte("test output"), | ||
| outputFile: "output.txt", | ||
| expectErr: false, | ||
| }, | ||
| { | ||
| name: "write to stdout", | ||
| data: []byte("test output"), | ||
| outputFile: "", | ||
| expectErr: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| if tt.outputFile != "" { | ||
| // Create temp directory for file output | ||
| tempDir := t.TempDir() | ||
| tt.outputFile = tempDir + "/" + tt.outputFile | ||
| } | ||
|
|
||
| err := writeOutput(tt.data, tt.outputFile) | ||
|
|
||
| if tt.expectErr { | ||
| assert.Error(t, err) | ||
| } else { | ||
| require.NoError(t, err) | ||
|
|
||
| if tt.outputFile != "" { | ||
| // Verify file was written | ||
| content, err := os.ReadFile(tt.outputFile) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, tt.data, content) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestStripANSICodes(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| input string | ||
| expected string | ||
| }{ | ||
| { | ||
| name: "text with ANSI codes", | ||
| input: "\x1b[31mred text\x1b[0m", | ||
| expected: "31mred text0m", | ||
| }, | ||
| { | ||
| name: "text without ANSI codes", | ||
| input: "plain text", | ||
| expected: "plain text", | ||
| }, | ||
| { | ||
| name: "empty string", | ||
| input: "", | ||
| expected: "", | ||
| }, | ||
| { | ||
| name: "multiple ANSI codes", | ||
| input: "\x1b[1m\x1b[31mbold red\x1b[0m\x1b[0m", | ||
| expected: "1m31mbold red0m0m", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| result := stripANSICodes([]byte(tt.input)) | ||
| assert.Equal(t, tt.expected, string(result)) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the git_diff.go file to see implementations
find . -name "git_diff.go" -type fRepository: cloudposse/atmos
Length of output: 87
🏁 Script executed:
cat -n ./pkg/vendoring/git_diff.goRepository: cloudposse/atmos
Length of output: 6378
🏁 Script executed:
# Let's verify the test expectations match the implementation
# Check what stripANSICodes actually produces for the test inputs
python3 << 'EOF'
import re
def stripANSICodes(data):
"""Mirrors the Go implementation"""
s = data
s = s.replace("\x1b[", "")
return s
# Test cases from the test file
test_cases = [
("\x1b[31mred text\x1b[0m", "31mred text0m"),
("plain text", "plain text"),
("", ""),
("\x1b[1m\x1b[31mbold red\x1b[0m\x1b[0m", "1m31mbold red0m0m"),
]
print("Verifying stripANSICodes test expectations:")
for input_str, expected in test_cases:
result = stripANSICodes(input_str)
matches = result == expected
print(f"Input: {repr(input_str)}")
print(f"Expected: {repr(expected)}")
print(f"Got: {repr(result)}")
print(f"Match: {matches}")
print()
EOFRepository: cloudposse/atmos
Length of output: 146
Use filepath.Join for cross-platform path compatibility
In TestWriteOutput, the path construction tempDir + "/" + tt.outputFile uses a hardcoded forward slash, which doesn't work correctly on Windows. Use filepath.Join(tempDir, tt.outputFile) instead to ensure consistent path handling across all platforms.
The stripANSICodes test expectations are correct—the function intentionally removes only the escape sequence prefix (\x1b[), leaving the numeric codes. This is working as designed and matches the implementation.
🤖 Prompt for AI Agents
In pkg/vendoring/git_diff_test.go around lines 162 to 243, the TestWriteOutput
constructs the temp file path using string concatenation tempDir + "/" +
tt.outputFile which is not cross-platform; change it to use
filepath.Join(tempDir, tt.outputFile) and ensure the test file imports
"path/filepath" (or adds it to the existing import block). The stripANSICodes
tests are fine and require no changes.
pkg/vendoring/source_provider.go
Outdated
| // GetProviderForSource returns the appropriate VendorSourceProvider for a given source URL. | ||
| func GetProviderForSource(source string) VendorSourceProvider { | ||
| defer perf.Track(nil, "exec.GetProviderForSource")() | ||
|
|
||
| // Determine provider type from source URL | ||
| if isGitHubSource(source) { | ||
| return NewGitHubSourceProvider() | ||
| } | ||
|
|
||
| // For all other Git sources, return a generic Git provider | ||
| // (which has limited functionality compared to GitHub) | ||
| if isGitSource(source) { | ||
| return NewGenericGitSourceProvider() | ||
| } | ||
|
|
||
| // For non-Git sources (OCI, HTTP, local), return unsupported provider | ||
| return NewUnsupportedSourceProvider() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check provider struct definitions and their fields
rg -nP --type=go 'type (GitHub|GenericGit|Unsupported)SourceProvider struct' pkg/vendoring/ -A 10Repository: cloudposse/atmos
Length of output: 2727
🏁 Script executed:
# Also check if there's any existing caching or singleton pattern in the codebase
rg -nP --type=go 'sync\.(Once|RWMutex)|cache' pkg/vendoring/ -B 2 -A 5Repository: cloudposse/atmos
Length of output: 42
🏁 Script executed:
# Check the provider interface to understand what methods might need state
rg -nP --type=go 'type VendorSourceProvider interface' pkg/vendoring/ -A 20Repository: cloudposse/atmos
Length of output: 1827
Factory function correctly selects providers, but GitHub provider creates expensive resources on every call.
The provider selection logic is clear: GitHub → Generic Git → Unsupported. The perf.Track is present and all comments end with periods.
However, GitHubSourceProvider holds an httpClient *http.Client which is created fresh on every GetProviderForSource call. HTTP clients maintain connection pools and goroutines; recreating one repeatedly is wasteful. Consider caching the provider instances using a module-level variable with sync.Once or a singleton pattern. The other providers (UnsupportedSourceProvider and GenericGitSourceProvider) are stateless, so caching them provides minimal benefit, but at minimum cache the GitHub provider.
🤖 Prompt for AI Agents
In pkg/vendoring/source_provider.go around lines 44 to 61, the factory currently
constructs a new GitHubSourceProvider (which allocates a fresh http.Client and
connection pool) on every call; change this to return a cached singleton
GitHubSourceProvider instead: add a module-level variable for the GitHub
provider and initialize it once (use sync.Once or similar) so the same instance
(and its http.Client) is reused across calls; keep GenericGitSourceProvider and
UnsupportedSourceProvider as-is or optionally cache them too, and ensure
GetProviderForSource returns the cached GitHub provider rather than creating a
new one each invocation.
- Fix go:generate package name in diff_integration_test.go (vendor -> vendoring) - Fix incomplete ANSI stripping with proper regex pattern - Fix pointer reassignment bug in utils.go (sourceURI = &absPath -> *sourceURI = absPath) - Remove debug fmt.Printf statements in update.go - Add periods to comment endings per godot linter requirements - Update PRD error naming to match code (ErrVendorConfigNotFound) - Update test expectations for correct ANSI stripping behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
pkg/vendoring/git_diff_test.go (1)
162-207: Use filepath.Join for cross-platform compatibility.Line 188 uses string concatenation with hardcoded forward slash. This breaks on Windows.
Apply this diff:
if tt.outputFile != "" { // Create temp directory for file output tempDir := t.TempDir() - tt.outputFile = tempDir + "/" + tt.outputFile + tt.outputFile = filepath.Join(tempDir, tt.outputFile) }As per coding guidelines: Use filepath.Join(), not hardcoded path separators.
🧹 Nitpick comments (5)
pkg/vendoring/git_diff.go (1)
111-168: Consider exposing GitDiffOptions to getGitDiffBetweenRefs.The function currently accepts individual parameters but doesn't support FilePath filtering or use the GitDiffOptions struct. To leverage file-scoped diffs from buildGitDiffArgs, consider refactoring the signature or adding a companion function that accepts GitDiffOptions.
Example approach:
+// getGitDiffBetweenRefsWithOptions generates a diff using GitDiffOptions configuration. +func getGitDiffBetweenRefsWithOptions(atmosConfig *schema.AtmosConfiguration, opts *GitDiffOptions) ([]byte, error) { + defer perf.Track(atmosConfig, "exec.getGitDiffBetweenRefsWithOptions")() + + tempDir, err := os.MkdirTemp("", "atmos-vendor-diff-*") + if err != nil { + return nil, fmt.Errorf("%w: %w", errUtils.ErrCreateTempDir, err) + } + defer os.RemoveAll(tempDir) + + // Initialize and fetch as before... + ctx := context.Background() + cmd := exec.CommandContext(ctx, "git", "init", "--bare", tempDir) + if err := cmd.Run(); err != nil { + return nil, fmt.Errorf("%w: %w", errUtils.ErrGitCommandFailed, err) + } + + cmd = exec.CommandContext(ctx, "git", "-C", tempDir, "fetch", "--depth=1", opts.GitURI, + opts.FromRef+":"+opts.FromRef, opts.ToRef+":"+opts.ToRef) + if err := cmd.Run(); err != nil { + return nil, fmt.Errorf("%w: failed to fetch refs: %w", errUtils.ErrGitCommandFailed, err) + } + + // Use buildGitDiffArgs to construct arguments + colorize := shouldColorizeOutput(opts.NoColor, opts.OutputFile) + args := []string{"-C", tempDir} + args = append(args, buildGitDiffArgs(opts, colorize)...) + + cmd = exec.CommandContext(ctx, "git", args...) + output, err := cmd.Output() + // ... error handling as before + return output, nil +}This would enable full GitDiffOptions support including FilePath filtering.
pkg/vendoring/update.go (2)
46-72: Wrap vendor-config read errors with context.Right now any error from
ReadAndProcessVendorConfigFileis returned as‑is. Consider adding brief context so callers can see which path failed while still preserving the sentinel via%w, for example:- vendorConfig, vendorConfigExists, foundVendorConfigFile, err := ReadAndProcessVendorConfigFile( + vendorConfig, vendorConfigExists, foundVendorConfigFile, err := ReadAndProcessVendorConfigFile( atmosConfig, vendorConfigFileName, true, ) if err != nil { - return err + return fmt.Errorf("reading vendor config %q: %w", vendorConfigFileName, err) }This keeps existing error semantics while improving debuggability.
As per coding guidelines, please confirm this aligns with the central error‑wrapping patterns in
errors/errors.gobefore changing multiple call sites.
100-115:splitAndTrimhelper is correct; tiny optional optimization.Logic for handling empty input, trimming, and skipping blanks is solid. If you ever want a micro‑optimization, you could preallocate the slice with
make([]string, 0, len(strings.Split(s, delimiter))), but it is not necessary for correctness.docs/prd/vendor-update.md (2)
903-975: Update architecture section to match the new vendoring layout.This block still references
internal/exec/vendor_*.goas the primary implementation surface, but the current PR is moving that logic intopkg/vendoringwithcmd/vendor/*wiring. It would be less confusing for future readers if the package tree and core function signatures here reflected the new structure (e.g.,pkg/vendoring/update.go,pkg/vendoring/diff.go, and their exported entrypoints), while still keeping the high‑level design intact.
64-78: Optionally add languages to output-only fenced code blocks.Several fenced blocks that show plain CLI/output snippets (e.g., “Checking for vendor updates…”, TUI states, data‑flow diagram paths, test fixture layout) omit a language, which triggers MD040 from markdownlint. If you want a clean lint run, consider tagging these as
textorbash(for shell transcripts) so all fences have an explicit language. Content itself is fine as‑is.Also applies to: 864-890, 979-1008, 1060-1068
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
docs/prd/vendor-update.md(1 hunks)pkg/vendoring/diff_integration_test.go(1 hunks)pkg/vendoring/git_diff.go(1 hunks)pkg/vendoring/git_diff_test.go(1 hunks)pkg/vendoring/update.go(1 hunks)pkg/vendoring/utils.go(19 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/vendoring/diff_integration_test.go
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
All comments must end with periods. This is enforced by the godot linter.
NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments.
Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly.
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Use viper.BindEnv with ATMOS_ prefix for environment variables.
Use colors from pkg/ui/theme/colors.go for theme consistency.
Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider us...
Files:
pkg/vendoring/git_diff_test.gopkg/vendoring/git_diff.gopkg/vendoring/utils.gopkg/vendoring/update.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% code coverage.
Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Files:
pkg/vendoring/git_diff_test.go
docs/prd/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Create PRD documentation in docs/prd/ using kebab-case filenames (e.g., command-registry-pattern.md, error-handling-strategy.md).
Files:
docs/prd/vendor-update.md
🧠 Learnings (52)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to {go.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
Applied to files:
pkg/vendoring/git_diff_test.gopkg/vendoring/update.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% code coverage.
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to tests/**/*_test.go : Never manually edit golden snapshot files. Always use -regenerate-snapshots flag. Snapshots capture exact output including invisible formatting (lipgloss padding, ANSI codes, trailing whitespace). Never use pipe redirection when running tests.
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Applied to files:
pkg/vendoring/git_diff_test.gopkg/vendoring/git_diff.gopkg/vendoring/utils.gopkg/vendoring/update.go
📚 Learning: 2024-12-25T20:28:19.618Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/workflow_utils.go:167-169
Timestamp: 2024-12-25T20:28:19.618Z
Learning: The user plans to revert the change from `path.Join` to `filepath.Join` in this PR due to testing gaps and will open a new PR to safely handle the migration without breaking `main`.
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/vendoring/git_diff_test.gopkg/vendoring/git_diff.gopkg/vendoring/utils.gopkg/vendoring/update.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
pkg/vendoring/git_diff_test.gopkg/vendoring/git_diff.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
pkg/vendoring/git_diff_test.gopkg/vendoring/git_diff.gopkg/vendoring/utils.gopkg/vendoring/update.go
📚 Learning: 2025-02-19T05:50:35.853Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden:0-0
Timestamp: 2025-02-19T05:50:35.853Z
Learning: Backtick formatting should only be applied to flag descriptions in Go source files, not in golden test files (test snapshots) as they are meant to capture the raw command output.
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Separate I/O (streams) from UI (formatting) using pkg/io/ and pkg/ui/ packages. Use data.Write/data.WriteJSON/data.WriteYAML for pipeable output (stdout), ui.Write/ui.Success/ui.Error for UI messages (stderr), and ui.Markdown for rendered documentation.
Applied to files:
pkg/vendoring/git_diff_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Applied to files:
pkg/vendoring/git_diff.gopkg/vendoring/utils.gopkg/vendoring/update.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
Applied to files:
pkg/vendoring/git_diff.gopkg/vendoring/update.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Applied to files:
pkg/vendoring/git_diff.go
📚 Learning: 2024-10-22T23:00:20.627Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Applied to files:
pkg/vendoring/utils.gopkg/vendoring/update.go
📚 Learning: 2024-10-31T07:09:31.983Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:181-182
Timestamp: 2024-10-31T07:09:31.983Z
Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.
Applied to files:
pkg/vendoring/utils.gopkg/vendoring/update.go
📚 Learning: 2025-02-05T11:10:51.031Z
Learnt from: mss
Repo: cloudposse/atmos PR: 1024
File: internal/exec/go_getter_utils.go:31-33
Timestamp: 2025-02-05T11:10:51.031Z
Learning: The path traversal check in `ValidateURI` function in `internal/exec/go_getter_utils.go` is intentionally kept despite potentially blocking valid Git URLs, as this validation is planned to be addressed in a separate ticket.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2024-10-23T20:13:23.054Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 731
File: pkg/utils/file_utils.go:198-202
Timestamp: 2024-10-23T20:13:23.054Z
Learning: In `pkg/utils/file_utils.go`, the current implementation of the `IsURL` function is considered sufficient; avoid suggesting more complex URL validation in future reviews.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
pkg/vendoring/utils.gopkg/vendoring/update.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2024-12-05T22:33:54.807Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_stacks.go:55-56
Timestamp: 2024-12-05T22:33:54.807Z
Learning: In the atmos project, the `u.LogErrorAndExit` function logs the error and exits the command execution appropriately within flag completion functions.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/vendoring/utils.gopkg/vendoring/update.godocs/prd/vendor-update.md
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2025-01-18T15:15:41.645Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
pkg/vendoring/utils.gopkg/vendoring/update.go
📚 Learning: 2024-10-20T13:06:20.839Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/utils/file_utils.go:177-194
Timestamp: 2024-10-20T13:06:20.839Z
Learning: In `pkg/utils/file_utils.go`, the `SearchConfigFile` function should focus on returning the file path and an `exists` boolean without returning an error.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2024-11-30T22:07:08.610Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2025-02-03T15:51:48.035Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 984
File: internal/exec/go_getter_utils.go:103-109
Timestamp: 2025-02-03T15:51:48.035Z
Learning: When checking for subdirectories in GitHub URLs, use `parsedURL.Path` to check for "//" instead of the entire URL, as the scheme portion (e.g., "https://") will always contain "//".
Applied to files:
pkg/vendoring/utils.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to {go.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2025-01-07T20:38:09.618Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 896
File: cmd/editor_config.go:37-40
Timestamp: 2025-01-07T20:38:09.618Z
Learning: Error handling suggestion for `cmd.Help()` in `cmd/editor_config.go` was deferred as the code is planned for future modifications.
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2024-11-10T19:28:17.365Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:0-0
Timestamp: 2024-11-10T19:28:17.365Z
Learning: When TTY is not supported, log the downgrade message at the Warn level using `u.LogWarning(cliConfig, ...)` instead of `fmt.Println`.
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2025-02-13T07:30:28.946Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: internal/exec/go_getter_utils.go:74-75
Timestamp: 2025-02-13T07:30:28.946Z
Learning: In the `CustomGitDetector.Detect` method of `internal/exec/go_getter_utils.go`, verbose debug logging of raw URLs is intentionally kept for debugging purposes, despite potential credential exposure risks.
Applied to files:
pkg/vendoring/update.go
📚 Learning: 2025-01-25T03:51:57.689Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Applied to files:
docs/prd/vendor-update.md
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
docs/prd/vendor-update.md
📚 Learning: 2024-10-31T01:22:09.586Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 737
File: examples/demo-vendoring/vendor.d/vendor1.yaml:10-11
Timestamp: 2024-10-31T01:22:09.586Z
Learning: In `examples/demo-vendoring/vendor.d/vendor1.yaml`, when specifying the source for the `ipinfo` component, it's acceptable to use `main` as the version reference if pinning to a specific version is not practical.
Applied to files:
docs/prd/vendor-update.md
📚 Learning: 2025-09-10T21:17:55.273Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Applied to files:
docs/prd/vendor-update.md
🧬 Code graph analysis (4)
pkg/vendoring/git_diff_test.go (1)
pkg/vendoring/git_diff.go (1)
GitDiffOptions(20-29)
pkg/vendoring/git_diff.go (3)
pkg/terminal/terminal.go (2)
Stdout(79-79)Stderr(80-80)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (3)
ErrCreateTempDir(25-25)ErrGitCommandFailed(128-128)ErrGitDiffFailed(575-575)
pkg/vendoring/utils.go (4)
errors/errors.go (13)
ErrVendoringNotConfigured(603-603)ErrPermissionDenied(604-604)ErrNoYAMLConfigFiles(607-607)ErrDuplicateComponentsFound(610-610)ErrMissingVendorConfigDefinition(602-602)ErrEmptySources(605-605)ErrNoComponentsWithTags(606-606)ErrDuplicateComponents(608-608)ErrVendorComponentNotDefinedInConfig(611-611)ErrDuplicateImport(609-609)ErrVendorConfigSelfImport(601-601)ErrSourceMissing(599-599)ErrTargetsMissing(600-600)internal/exec/template_utils.go (1)
ProcessTmpl(55-99)pkg/vendoring/uri/helpers.go (9)
IsFileURI(18-22)IsOCIURI(25-29)IsS3URI(33-37)IsLocalPath(83-120)IsNonGitHTTPURI(138-153)ContainsTripleSlash(232-238)NeedsDoubleSlashDot(259-279)AppendDoubleSlashDot(283-303)ParseSubdirFromTripleSlash(245-256)pkg/utils/file_utils.go (2)
JoinPathAndValidate(175-196)FileExists(31-39)
pkg/vendoring/update.go (6)
pkg/schema/schema.go (2)
AtmosConfiguration(53-94)Vendor(941-946)pkg/vendoring/params.go (1)
UpdateParams(16-23)pkg/perf/perf.go (1)
Track(121-138)pkg/config/const.go (1)
AtmosVendorConfigFileName(62-62)pkg/vendoring/utils.go (1)
ReadAndProcessVendorConfigFile(67-97)errors/errors.go (2)
ErrVendorConfigNotFound(574-574)ErrNotImplemented(29-29)
🪛 LanguageTool
docs/prd/vendor-update.md
[typographical] ~7-~7: To join two clauses or introduce examples, consider using an em dash.
Context: ...anagement: 1. atmos vendor update - Automated version management for vendore...
(DASH_RULE)
[typographical] ~9-~9: To join two clauses or introduce examples, consider using an em dash.
Context: ... formatting. 2. atmos vendor diff - Shows Git diffs between two versions of ...
(DASH_RULE)
[typographical] ~185-~185: To join two clauses or introduce examples, consider using an em dash.
Context: ... Tags & commits via git ls-remote | P0 - MUST | Primary use case | | **OCI Regist...
(DASH_RULE)
[typographical] ~186-~186: To join two clauses or introduce examples, consider using an em dash.
Context: ...| OCI Registries | Registry API | P2 - Future | Complex, different API per regi...
(DASH_RULE)
[typographical] ~189-~189: To join two clauses or introduce examples, consider using an em dash.
Context: ...| | Amazon S3 | Object metadata | P3 - Future | Requires AWS SDK | | **Google G...
(DASH_RULE)
[typographical] ~190-~190: To join two clauses or introduce examples, consider using an em dash.
Context: ... | Google GCS | Object metadata | P3 - Future | Requires GCP SDK | ### Git Rep...
(DASH_RULE)
[grammar] ~227-~227: Please add a punctuation mark at the end of paragraph.
Context: ...r constraints familiar from npm, cargo, composer ### Configuration Schema Constraints ...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~265-~265: Consider using a typographic close quote here.
Context: ...ration. Supports: - Exact versions: "1.2.3" - exclude this specific version ...
(EN_QUOTES)
[typographical] ~265-~265: To join two clauses or introduce examples, consider using an em dash.
Context: ...upports: - Exact versions: "1.2.3" - exclude this specific version - **Wildca...
(DASH_RULE)
[typographical] ~266-~266: Consider using a typographic close quote here.
Context: ...ecific version - Wildcard patterns: "1.5.*" - exclude all 1.5.x versions - *...
(EN_QUOTES)
[typographical] ~266-~266: To join two clauses or introduce examples, consider using an em dash.
Context: ...rsion - Wildcard patterns: "1.5.*" - exclude all 1.5.x versions - **Multiple ...
(DASH_RULE)
[typographical] ~277-~277: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...se versions (alpha, beta, rc, pre, etc.). **Examples of excluded versions when `...
(WRB_QUESTION_MARK)
[uncategorized] ~293-~293: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ...ons** - skip non-semver tags 3. Apply no_prereleases filter - remove alpha/beta/rc if ena...
(EN_WORD_COHERENCY)
[typographical] ~409-~409: Consider using a typographic close quote here.
Context: ...Quote Styles: Single ('), double ("), or unquoted 7. Line Ordering: Ma...
(EN_QUOTES)
[typographical] ~415-~415: To join two clauses or introduce examples, consider using an em dash.
Context: ...rategy Use Established YAML Libraries - DO NOT implement custom parser **Reco...
(DASH_RULE)
[grammar] ~425-~425: Please add a punctuation mark at the end of paragraph.
Context: ...in the Go ecosystem - Battle-tested and maintained Implementation: ```go import "gopk...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~452-~452: Please add a punctuation mark at the end of paragraph.
Context: ...mitations** and file issues upstream if needed Key Requirements: 1. Use `yaml.Nod...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~459-~459: Please add a punctuation mark at the end of paragraph.
Context: ...ering 5. Test with complex anchor/alias scenarios ### Test Cases for YAML Preservation ...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~522-~522: To join two clauses or introduce examples, consider using an em dash.
Context: ...nts - --component <name> / -c <name> - Update specific component only - `--tags...
(DASH_RULE)
[typographical] ~524-~524: To join two clauses or introduce examples, consider using an em dash.
Context: ...parated) - --type <type> / -t <type> - Component type: terraform or `helmfile...
(DASH_RULE)
[typographical] ~557-~557: To join two clauses or introduce examples, consider using an em dash.
Context: ...:** - --component <name> / -c <name> - Component to diff (required) - `--from <...
(DASH_RULE)
[typographical] ~629-~629: Consider using typographic quotation marks here.
Context: ...itLab, Bitbucket, self-hosted), returns "not implemented" - Not applicable to OCI registries, loc...
(EN_QUOTES)
[typographical] ~894-~894: To join two clauses or introduce examples, consider using an em dash.
Context: ...3-bucket... ``` ### Status Icons - ✓ - Update available or operation succeeded ...
(DASH_RULE)
[typographical] ~1110-~1110: Consider using a typographic opening quote here.
Context: ...ndor.yaml | ErrVendorConfigNotFound | "Vendor config file not found: vendor.yam...
(EN_QUOTES)
[typographical] ~1110-~1110: Consider using a typographic close quote here.
Context: ...endor config file not found: vendor.yaml" | Check file path | | Git ls-remote fai...
(EN_QUOTES)
[typographical] ~1111-~1111: Consider using a typographic opening quote here.
Context: ...emote fails | ErrCheckingForUpdates | "Failed to check updates for vpc: connect...
(EN_QUOTES)
[typographical] ~1111-~1111: Consider using a typographic close quote here.
Context: ...heck updates for vpc: connection timeout" | Check network, try again | | No tags ...
(EN_QUOTES)
[typographical] ~1112-~1112: Consider using a typographic opening quote here.
Context: ... | No tags in repo | ErrNoTagsFound | "No version tags found in repository" | U...
(EN_QUOTES)
[typographical] ~1112-~1112: Consider using a typographic close quote here.
Context: ...` | "No version tags found in repository" | Use commit hash versioning | | Rate l...
(EN_QUOTES)
[typographical] ~1113-~1113: Consider using a typographic opening quote here.
Context: ...it hit | ErrGitHubRateLimitExceeded | "GitHub rate limit exceeded. Use SSH or s...
(EN_QUOTES)
[typographical] ~1113-~1113: Consider using a typographic close quote here.
Context: ...t exceeded. Use SSH or set GITHUB_TOKEN." | Wait or authenticate | | Invalid vers...
(EN_QUOTES)
[typographical] ~1114-~1114: Consider using a typographic opening quote here.
Context: ...format | errUtils.ErrInvalidVersion | "Invalid version format: 'abc'. Expected ...
(EN_QUOTES)
[typographical] ~1114-~1114: Consider using a typographic close quote here.
Context: ...: 'abc'. Expected semver or commit hash." | Fix version string | | Templated vers...
(EN_QUOTES)
[typographical] ~1115-~1115: Consider using a typographic opening quote here.
Context: ...| Templated version | (skip silently) | "⚠ vpc (skipped - templated version)" | N...
(EN_QUOTES)
[typographical] ~1115-~1115: To join two clauses or introduce examples, consider using an em dash.
Context: ...sion | (skip silently) | "⚠ vpc (skipped - templated version)" | No action needed |...
(DASH_RULE)
[typographical] ~1115-~1115: Consider using a typographic close quote here.
Context: ...) | "⚠ vpc (skipped - templated version)" | No action needed | | YAML parse error...
(EN_QUOTES)
[typographical] ~1116-~1116: Consider using a typographic opening quote here.
Context: ... YAML parse error | yaml.ParseError | "Failed to parse vendor.yaml: line 5, inv...
(EN_QUOTES)
[typographical] ~1116-~1116: Consider using a typographic close quote here.
Context: ...arse vendor.yaml: line 5, invalid syntax" | Fix YAML syntax | ## Documentation R...
(EN_QUOTES)
[typographical] ~1210-~1210: To join two clauses or introduce examples, consider using an em dash.
Context: ...ses ### Phase 1: Core Functionality (v1 - MVP) Goal: Complete vendor managemen...
(DASH_RULE)
[typographical] ~1309-~1309: Consider using typographic quotation marks here.
Context: ...ons using GitHub's Compare API; returns "not implemented" for non-GitHub sources ## Migration Pa...
(EN_QUOTES)
[typographical] ~1386-~1386: Consider using a typographic opening quote here.
Context: ... explicit version field - Show warning: "⚠ vpc (skipped - no version field)" - Do...
(EN_QUOTES)
[typographical] ~1386-~1386: To join two clauses or introduce examples, consider using an em dash.
Context: ...on field - Show warning: "⚠ vpc (skipped - no version field)" - Don't add version f...
(DASH_RULE)
[typographical] ~1386-~1386: Consider using a typographic close quote here.
Context: ...ing: "⚠ vpc (skipped - no version field)" - Don't add version field automatically...
(EN_QUOTES)
[grammar] ~1399-~1399: Please add a punctuation mark at the end of paragraph.
Context: ...xec/vendor_filter.gofor component/tag filtering **Vendor Diff:** - [ ] Createcmd/ven...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~1470-~1470: To join two clauses or introduce examples, consider using an em dash.
Context: ...anagement: 1. atmos vendor update - Automates version checking and YAML upda...
(DASH_RULE)
[uncategorized] ~1485-~1485: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...) and documentation requirements ensure high quality and excellent user experience from day ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/vendor-update.md
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
415-415: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
708-708: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
715-715: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
721-721: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
727-727: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
767-767: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
864-864: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
871-871: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
879-879: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
886-886: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
903-903: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
979-979: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1060-1060: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (16)
pkg/vendoring/utils.go (6)
50-55: LGTM on sourceTypeResult struct.Clean, focused type with clear field names. The struct provides a unified return for source type classification.
519-553: Pointer mutation updates the caller correctly.The change from
sourceURI = &absPathto*sourceURI = absPathon line 535 properly updates the caller's string value. This addresses the past review concern.
468-486: URI normalization logic is well-structured.The function appropriately skips special URI types and delegates to helper functions. The flow is clear and maintainable.
490-517: Triple-slash normalization handles edge cases correctly.Query parameter preservation and subdirectory handling logic are solid. The debug logging provides good observability.
350-350: Consistent template processing across targets.The switch to
exec.ProcessTmplfor target processing matches the pattern used for source URIs on line 291. Good consistency.
290-291: Verify ProcessTmpl signature and template data structure compatibility.Confirm that
ProcessTmplis the correct template processor for this vendoring use case and thattmplDatamatches the expected parameter signature. Ensure the function is appropriately scoped for processing vendor source templates with the given parameters.pkg/vendoring/git_diff_test.go (3)
11-96: Comprehensive test coverage for buildGitDiffArgs.Table-driven tests cover color options, file filters, context variations, and unified format. Well-structured test cases.
98-160: TTY-dependent tests handled appropriately.The CI skip logic on line 147-149 prevents flaky tests in non-TTY environments. Good defensive testing.
209-243: ANSI stripping tests verify expected behavior.The test expectations correctly validate the stripANSICodes implementation. Multiple test cases provide good coverage.
pkg/vendoring/git_diff.go (5)
19-29: Well-defined options struct.The GitDiffOptions struct provides clear configuration for diff operations. FilePath field enables file-scoped diffs as requested.
32-62: buildGitDiffArgs correctly handles file filtering.Lines 57-59 properly append the file path filter with the
--separator, addressing the past review concern about file-scoped diffs.
69-93: Colorization logic is thorough.The function checks no-color flag, output file, TERM variable, and TTY status. Line 81 properly uses viper.BindEnv with ATMOS_ prefix. Good environmental awareness.
170-176: ANSI regex implementation correctly strips escape codes.The regex pattern
\x1b\[[0-9;]*mproperly matches complete SGR sequences, addressing the past review concern. This is a solid fix.
112-112: Performance tracking is properly instrumented.The defer perf.Track call follows the coding guideline pattern for public functions. Good adherence to standards.
pkg/vendoring/update.go (2)
13-21: Struct design forupdateFlagslooks good.Field set matches
UpdateParamsand future filtering needs; no changes needed here.
23-44:Updateentrypoint wiring is clean and consistent.Params →
updateFlagsconversion and perf tracking look good, and tag parsing viasplitAndTrimmatches the--tagscomma‑separated contract.
| **File:** `website/docs/cli/commands/vendor/vendor-diff.mdx` | ||
|
|
||
| **Sections:** | ||
| - Purpose note (alias to vendor update --check) | ||
| - Usage syntax | ||
| - Link to vendor-update.mdx for details | ||
|
|
There was a problem hiding this comment.
Clarify that vendor diff is a distinct command, not an alias.
The CLI docs checklist describes the vendor‑diff page as “alias to vendor update --check”, but elsewhere this PRD (and the implementation) treat atmos vendor diff as a separate Git diff command with its own flags and behavior. I’d rephrase this note to describe vendor‑diff’s purpose (Git diffs between component versions) and link to vendor-update only for shared background, to avoid implying they are the same operation.
🤖 Prompt for AI Agents
In docs/prd/vendor-update.md around lines 1135 to 1141, the note incorrectly
labels the CLI page vendor-diff as an alias of "vendor update --check"; update
the copy to state that "atmos vendor diff" is a distinct command that performs
Git diffs between component versions and has its own flags and behavior, and
change the link to vendor-update to be a "see also / shared background"
reference rather than an alias. Ensure the wording describes vendor-diff's
purpose (compare component versions via Git diffs), lists that it has separate
flags, and only references vendor-update for related context.
| if !vendorConfigExists { | ||
| // Try component vendor config if no main vendor config. | ||
| if flags.Component != "" { | ||
| return executeComponentVendorUpdate(atmosConfig, flags) | ||
| } | ||
| return fmt.Errorf("%w: %s", errUtils.ErrVendorConfigNotFound, vendorConfigFileName) | ||
| } | ||
|
|
||
| // TODO: Implement actual update logic. | ||
| // 1. Process imports and get sources. | ||
| // 2. Filter sources by component/tags. | ||
| // 3. Check for updates using Git. | ||
| // 4. Display results (TUI). | ||
| // 5. Update YAML files if not --check. | ||
| // 6. Execute vendor pull if --pull. | ||
|
|
||
| // Use vendorConfig and foundVendorConfigFile to avoid "declared and not used" error. | ||
| _ = vendorConfig | ||
| _ = foundVendorConfigFile | ||
|
|
||
| return errUtils.ErrNotImplemented | ||
| } |
There was a problem hiding this comment.
Vendor-update flow is still a stub and will currently always fail.
If vendor.yaml (or equivalent) exists, executeVendorUpdate ends by returning errUtils.ErrNotImplemented, and the TODO covers all the core logic (imports, filtering, Git checks, YAML updates, optional pull). As wired from Update, this means atmos vendor update cannot yet perform any real work.
Before merging the feature, this function needs to be hooked up to the actual vendoring pipeline (sources/import resolution, version checks, YAML updater, TUI, optional pull) so callers don’t just see not implemented. The temporary _ = vendorConfig / _ = foundVendorConfigFile lines can be dropped once real logic consumes these values.
If helpful, I can sketch how to thread the existing ReadAndProcessVendorConfigFile output into the planned YAML‑updater + Git‑checker interfaces described elsewhere in this PR.
| // executeComponentVendorUpdate handles vendor update for component.yaml files. | ||
| func executeComponentVendorUpdate(atmosConfig *schema.AtmosConfiguration, flags *updateFlags) error { | ||
| defer perf.Track(atmosConfig, "vendor.executeComponentVendorUpdate")() | ||
|
|
||
| // TODO: Implement component vendor update. | ||
| // Use flags.Component and flags.ComponentType (default: "terraform") to avoid unused variable warning. | ||
| _ = flags | ||
|
|
||
| return errUtils.ErrNotImplemented | ||
| } |
There was a problem hiding this comment.
Component-level vendor update is also a placeholder.
executeComponentVendorUpdate currently ignores flags and returns ErrNotImplemented. That is fine as an internal scaffold, but as soon as --component is exposed in the CLI this needs to route into the real component‑level update logic (e.g., component.yaml handling) rather than surfacing not implemented at runtime.
I’d treat this as part of the same implementation task as the main executeVendorUpdate TODO, to avoid partially working flags.
Happy to help outline the component‑specific path (type selection, component.yaml discovery) once the shared vendoring utilities are in place.
Reorganize pkg/vendoring into focused subpackages for better maintainability: - pkg/vendoring/source/: Source provider abstraction (GitHub, Git, unsupported) - Provider interface with GetAvailableVersions, VerifyVersion, GetDiff - GitHubProvider with GitHub API integration - GenericGitProvider for non-GitHub Git sources - UnsupportedProvider for OCI/local sources - pkg/vendoring/version/: Version management and constraints - GetGitRemoteTags, CheckGitRef for Git operations - ParseSemVer, FindLatestSemVerTag for version comparison - ResolveVersionConstraints with semver filtering - ExtractGitURI for source URL parsing Updates diff.go and git_interface.go to use new subpackages. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
…update # Conflicts: # website/docs/core-concepts/vendor/vendor-manifest.mdx
- Path-escape ref names in GitHub Compare API URL to handle refs with slashes - Implement GetGitHubToken using viper.BindEnv to check ATMOS_GITHUB_TOKEN first, then GITHUB_TOKEN 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
pkg/vendoring/version/constraints.go (1)
112-129: Wildcard matching may have edge case issues.The current implementation handles
*only at the end of patterns. Patterns like1.*.0or*-alphawon't work as expected sinceTrimSuffix(pattern, "*")only removes trailing*.If only suffix wildcards are intended, consider documenting this explicitly. Otherwise, a more robust pattern matching approach may be needed.
// MatchesWildcard checks if a version matches a wildcard pattern. -// Supports patterns like "1.5.*" or exact matches like "1.2.3". +// Supports patterns like "1.5.*" (suffix wildcards only) or exact matches like "1.2.3". +// Note: Wildcards are only supported at the end of patterns. func MatchesWildcard(version, pattern string) bool {pkg/vendoring/git_interface.go (2)
27-32: Perf track name prefix mismatch.The track names use
exec.prefix (e.g.,exec.NewGitOperations) but this file is in thevendoringpackage. Consider usingvendoring.prefix for consistency.func NewGitOperations() GitOperations { - defer perf.Track(nil, "exec.NewGitOperations")() + defer perf.Track(nil, "vendoring.NewGitOperations")() return &realGitOperations{} }
34-55: Same naming prefix issue applies here.Lines 36, 43, and 52 also use
exec.prefix. Update tovendoring.for consistency with the package name.Otherwise, the delegation pattern and interface design look solid.
pkg/vendoring/diff.go (3)
27-28: Perf track prefix should match package name.The package is
vendoringbut track names usevendor.prefix.func Diff(atmosConfig *schema.AtmosConfiguration, params *DiffParams) error { - defer perf.Track(atmosConfig, "vendor.Diff")() + defer perf.Track(atmosConfig, "vendoring.Diff")()
117-120: Use static error for wrapping.Per coding guidelines, errors should be wrapped using static errors from errors/errors.go.
+ // In errors/errors.go, add: + // ErrGetRemoteTagsFailed = errors.New("failed to get remote tags") tags, err := gitOps.GetRemoteTags(gitURI) if err != nil { - return fmt.Errorf("failed to get remote tags: %w", err) + return fmt.Errorf("%w: %w", errUtils.ErrGetRemoteTagsFailed, err) }
152-164: TODO: Component vendor diff not implemented.This stub correctly returns
ErrNotImplemented. The TODO outlines the expected behavior well.Want me to draft an implementation for this function or open an issue to track it?
pkg/vendoring/source/git.go (1)
58-70: Consider removing perf.Track from trivial accessor.
SupportsOperationis a simple O(1) switch statement. Per repository learnings, trivial accessors where tracking overhead exceeds operation cost should skip perf.Track.Same applies to
IsGitSource(line 74) which is just prefix/suffix checks.pkg/vendoring/version/check.go (2)
132-139: Precompile regex for performance.The regex is compiled on every call. Move it to package level for efficiency.
+var commitSHARegex = regexp.MustCompile(`^[0-9a-f]{7,40}$`) + // IsValidCommitSHA checks if a string looks like a valid Git commit SHA. func IsValidCommitSHA(ref string) bool { - defer perf.Track(nil, "version.IsValidCommitSHA")() - // Full SHA: 40 hex chars, short SHA: 7-40 hex chars. - matched, _ := regexp.MatchString(`^[0-9a-f]{7,40}$`, ref) - return matched + return commitSHARegex.MatchString(ref) }Also, perf.Track can be removed here per repository learnings about trivial operations.
141-163: Consider removing perf.Track from trivial function.
ExtractGitURIis just string manipulation. Per repository learnings, tracking overhead on trivial operations pollutes performance reports.pkg/vendoring/source/github.go (4)
20-28: Solid foundation for the provider.The struct is appropriately simple. One small nit: comment mentions "DefaultHTTPTimeout" (exported) but the constant is
defaultHTTPTimeout(unexported). Consider aligning them.const ( - // DefaultHTTPTimeout is the default timeout for HTTP requests to GitHub API. + // defaultHTTPTimeout is the default timeout for HTTP requests to GitHub API. defaultHTTPTimeout = 30 * time.Second )
58-119: Well-structured GitHub diff implementation.Good use of
url.PathEscapefor refs containing slashes. Error handling wraps domain errors properly and includes response body for debugging.The TODOs on lines 114-116 indicate pending work for file filtering, context lines, and color stripping. Want me to generate implementations for these features or open issues to track them?
133-166: Robust URL parsing logic.Handles git::, https, http, SSH formats, subpaths, and query params well.
One consideration: the error message at line 162 shows the transformed
sourcestring rather than the original input, which could confuse users trying to debug. Consider preserving the original for error context.func ParseGitHubRepo(source string) (owner, repo string, err error) { defer perf.Track(nil, "source.ParseGitHubRepo")() + originalSource := source // Remove common prefixes. source = strings.TrimPrefix(source, "git::") // ... rest of transformations ... // Split into owner/repo. parts := strings.SplitN(source, "/", 2) if len(parts) != 2 { - return "", "", fmt.Errorf("%w: invalid GitHub repository format: %s", errUtils.ErrParseURL, source) + return "", "", fmt.Errorf("%w: invalid GitHub repository format: %s", errUtils.ErrParseURL, originalSource) }
198-223: Rate limit check well-implemented.Error wrapping and perf tracking addressed from past review. Consider adding a status code check before JSON decoding - if the API returns an error status, the decode failure message might be confusing.
resp, err := g.httpClient.Do(req) if err != nil { return nil, fmt.Errorf("%w: failed to execute rate limit request: %w", errUtils.ErrHTTPRequestFailed, err) } defer resp.Body.Close() +if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return nil, fmt.Errorf("%w: rate limit API returned status %d: %s", errUtils.ErrHTTPRequestFailed, resp.StatusCode, string(body)) +} + var rateLimit GitHubRateLimitResponse
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
go.mod(1 hunks)pkg/vendoring/diff.go(1 hunks)pkg/vendoring/git_interface.go(1 hunks)pkg/vendoring/mock_git_interface.go(1 hunks)pkg/vendoring/source/git.go(1 hunks)pkg/vendoring/source/github.go(1 hunks)pkg/vendoring/source/mock_provider.go(1 hunks)pkg/vendoring/source/provider.go(1 hunks)pkg/vendoring/source/provider_test.go(1 hunks)pkg/vendoring/source/unsupported.go(1 hunks)pkg/vendoring/version/check.go(1 hunks)pkg/vendoring/version/check_test.go(1 hunks)pkg/vendoring/version/constraints.go(1 hunks)pkg/vendoring/version/constraints_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pkg/vendoring/version/constraints_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- go.mod
- pkg/vendoring/mock_git_interface.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
All comments must end with periods. This is enforced by the godot linter.
NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments.
Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly.
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Use viper.BindEnv with ATMOS_ prefix for environment variables.
Use colors from pkg/ui/theme/colors.go for theme consistency.
Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider us...
Files:
pkg/vendoring/diff.gopkg/vendoring/source/provider_test.gopkg/vendoring/git_interface.gopkg/vendoring/source/unsupported.gopkg/vendoring/source/git.gopkg/vendoring/version/constraints.gopkg/vendoring/version/check_test.gopkg/vendoring/source/provider.gopkg/vendoring/version/check.gopkg/vendoring/source/github.gopkg/vendoring/source/mock_provider.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% code coverage.
Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Files:
pkg/vendoring/source/provider_test.gopkg/vendoring/version/check_test.go
🧠 Learnings (34)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
Applied to files:
pkg/vendoring/diff.gopkg/vendoring/git_interface.gopkg/vendoring/version/constraints.gopkg/vendoring/source/provider.gopkg/vendoring/source/github.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/vendoring/diff.gopkg/vendoring/source/provider.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Applied to files:
pkg/vendoring/diff.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/vendoring/diff.gopkg/vendoring/git_interface.gopkg/vendoring/version/constraints.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/vendoring/source/provider_test.gopkg/vendoring/version/check_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% code coverage.
Applied to files:
pkg/vendoring/source/provider_test.gopkg/vendoring/version/check_test.gopkg/vendoring/source/mock_provider.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/vendoring/source/provider_test.gopkg/vendoring/version/check_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
Applied to files:
pkg/vendoring/source/provider_test.gopkg/vendoring/version/check_test.gopkg/vendoring/source/mock_provider.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/vendoring/source/provider_test.gopkg/vendoring/version/check_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
Applied to files:
pkg/vendoring/source/provider_test.gopkg/vendoring/version/check_test.go
📚 Learning: 2025-11-10T23:23:39.771Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/registry/aqua/aqua_test.go:417-442
Timestamp: 2025-11-10T23:23:39.771Z
Learning: In Atmos toolchain AquaRegistry, tests should not hit real GitHub. Use the options pattern via WithGitHubBaseURL to inject an httptest server URL and make GetLatestVersion/GetAvailableVersions deterministic.
Applied to files:
pkg/vendoring/source/provider_test.gopkg/vendoring/version/check_test.gopkg/vendoring/version/check.gopkg/vendoring/source/github.go
📚 Learning: 2025-11-30T04:16:24.155Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:34-48
Timestamp: 2025-11-30T04:16:24.155Z
Learning: In the cloudposse/atmos repository, the `defer perf.Track()` guideline applies to functions that perform meaningful work (I/O, computation, external calls), but explicitly excludes trivial accessors/mutators (e.g., simple getters, setters with single integer increments, string joins, or map appends) where the tracking overhead would exceed the actual method cost and provide no actionable performance data. Hot-path methods called in tight loops should especially avoid perf.Track() if they perform only trivial operations.
Applied to files:
pkg/vendoring/git_interface.gopkg/vendoring/version/constraints.gopkg/vendoring/source/provider.gopkg/vendoring/source/github.go
📚 Learning: 2025-11-30T04:16:01.899Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:50-59
Timestamp: 2025-11-30T04:16:01.899Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` should NOT be added to trivial O(1) getter methods that only return field references or check map lengths (e.g., `GetDeferredValues()`, `HasDeferredValues()`). The guideline to add perf tracking to "all public functions" applies to functions that do meaningful work (I/O, computation, external calls), not to trivial accessors where the tracking overhead would exceed the operation time and pollute performance reports.
Applied to files:
pkg/vendoring/git_interface.gopkg/vendoring/version/constraints.gopkg/vendoring/source/provider.gopkg/vendoring/source/github.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
pkg/vendoring/git_interface.gopkg/vendoring/version/constraints.gopkg/vendoring/source/provider.gopkg/vendoring/source/github.go
📚 Learning: 2025-02-05T11:10:51.031Z
Learnt from: mss
Repo: cloudposse/atmos PR: 1024
File: internal/exec/go_getter_utils.go:31-33
Timestamp: 2025-02-05T11:10:51.031Z
Learning: The path traversal check in `ValidateURI` function in `internal/exec/go_getter_utils.go` is intentionally kept despite potentially blocking valid Git URLs, as this validation is planned to be addressed in a separate ticket.
Applied to files:
pkg/vendoring/git_interface.gopkg/vendoring/version/check_test.go
📚 Learning: 2025-03-25T12:23:42.649Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:104-104
Timestamp: 2025-03-25T12:23:42.649Z
Learning: Listener430 plans to add a test for verifying that token injection is skipped for unsupported hosts in a future review or refactoring iteration. This relates to the CustomGitDetector.Detect method in internal/exec/go_getter_utils.go.
Applied to files:
pkg/vendoring/git_interface.go
📚 Learning: 2025-02-13T07:30:28.946Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: internal/exec/go_getter_utils.go:74-75
Timestamp: 2025-02-13T07:30:28.946Z
Learning: In the `CustomGitDetector.Detect` method of `internal/exec/go_getter_utils.go`, verbose debug logging of raw URLs is intentionally kept for debugging purposes, despite potential credential exposure risks.
Applied to files:
pkg/vendoring/git_interface.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
pkg/vendoring/git_interface.go
📚 Learning: 2024-10-22T23:00:20.627Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Applied to files:
pkg/vendoring/git_interface.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
pkg/vendoring/git_interface.gopkg/vendoring/version/constraints.gopkg/vendoring/source/provider.gopkg/vendoring/source/github.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using `fmt.Errorf("context: %w", err)`, and consider using custom error types for domain-specific errors
Applied to files:
pkg/vendoring/git_interface.gopkg/vendoring/source/github.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
pkg/vendoring/git_interface.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Use interface-driven design with dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen using //go:generate directives.
Applied to files:
pkg/vendoring/git_interface.gopkg/vendoring/source/mock_provider.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to {go.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Applied to files:
pkg/vendoring/version/constraints.gopkg/vendoring/version/check.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Ensure all tests pass, verify code coverage meets targets, run golangci-lint and fix any issues, and update documentation before submitting pull requests
Applied to files:
pkg/vendoring/version/check_test.go
📚 Learning: 2024-10-23T20:13:23.054Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 731
File: pkg/utils/file_utils.go:198-202
Timestamp: 2024-10-23T20:13:23.054Z
Learning: In `pkg/utils/file_utils.go`, the current implementation of the `IsURL` function is considered sufficient; avoid suggesting more complex URL validation in future reviews.
Applied to files:
pkg/vendoring/version/check_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Follow multi-provider registry pattern: 1) Define interface in dedicated package, 2) Implement per provider, 3) Register implementations, 4) Generate mocks. See pkg/store/ for reference (AWS SSM, Azure Key Vault, Google Secret Manager).
Applied to files:
pkg/vendoring/source/provider.gopkg/vendoring/source/mock_provider.go
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
pkg/vendoring/version/check.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
pkg/vendoring/version/check.go
📚 Learning: 2024-12-05T02:48:53.818Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 809
File: cmd/cmd_utils.go:470-477
Timestamp: 2024-12-05T02:48:53.818Z
Learning: The function `GetLatestGitHubRepoRelease` in the Go codebase already uses a context with a timeout, so wrapping it with an additional context is unnecessary.
Applied to files:
pkg/vendoring/source/github.go
📚 Learning: 2024-12-15T10:20:08.436Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/cmd_utils.go:454-464
Timestamp: 2024-12-15T10:20:08.436Z
Learning: Avoid adding timeout handling for GitHub API calls in `CheckForAtmosUpdateAndPrintMessage` function in `cmd/cmd_utils.go`, as it might be disabled by user settings.
Applied to files:
pkg/vendoring/source/github.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
Applied to files:
pkg/vendoring/source/github.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Applied to files:
pkg/vendoring/source/mock_provider.go
🧬 Code graph analysis (8)
pkg/vendoring/git_interface.go (2)
pkg/perf/perf.go (1)
Track(121-138)pkg/vendoring/version/check.go (2)
GetGitRemoteTags(28-59)CheckGitRef(96-130)
pkg/vendoring/source/unsupported.go (4)
pkg/vendoring/source/provider.go (2)
Provider(12-25)Operation(28-28)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (1)
ErrUnsupportedVendorSource(578-578)pkg/schema/schema.go (1)
AtmosConfiguration(53-94)
pkg/vendoring/source/git.go (3)
pkg/vendoring/source/provider.go (2)
Provider(12-25)Operation(28-28)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (1)
ErrNotImplemented(29-29)
pkg/vendoring/version/constraints.go (3)
pkg/schema/schema.go (1)
VendorConstraints(909-913)pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (3)
ErrNoVersionsAvailable(588-588)ErrNoVersionsMatchConstraints(589-589)ErrInvalidSemverConstraint(590-590)
pkg/vendoring/version/check_test.go (1)
pkg/vendoring/version/check.go (4)
ParseSemVer(62-70)FindLatestSemVerTag(73-93)IsValidCommitSHA(133-139)ExtractGitURI(143-163)
pkg/vendoring/source/provider.go (5)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/perf/perf.go (1)
Track(121-138)pkg/vendoring/source/github.go (2)
IsGitHubSource(169-173)NewGitHubProvider(31-39)pkg/vendoring/source/git.go (2)
IsGitSource(73-81)NewGenericGitProvider(18-22)pkg/vendoring/source/unsupported.go (1)
NewUnsupportedProvider(16-20)
pkg/vendoring/version/check.go (2)
pkg/perf/perf.go (1)
Track(121-138)errors/errors.go (1)
ErrGitLsRemoteFailed(579-579)
pkg/vendoring/source/github.go (4)
pkg/vendoring/source/provider.go (2)
Provider(12-25)Operation(28-28)pkg/perf/perf.go (1)
Track(121-138)pkg/vendoring/version/check.go (3)
ExtractGitURI(143-163)GetGitRemoteTags(28-59)CheckGitRef(96-130)errors/errors.go (5)
ErrGitDiffFailed(575-575)ErrParseURL(20-20)ErrFailedToCreateRequest(371-371)ErrHTTPRequestFailed(169-169)ErrFailedToUnmarshalAPIResponse(390-390)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Summary
🔇 Additional comments (24)
pkg/vendoring/source/unsupported.go (1)
1-58: Clean Provider implementation for unsupported sources.The
UnsupportedProvidercorrectly implements theProviderinterface, returning appropriate errors with proper wrapping viafmt.Errorfand%w. Theperf.Trackinstrumentation and error sentinel usage align with project conventions.pkg/vendoring/version/constraints.go (2)
154-184: Solid fallback behavior in SelectLatestVersion.The fallback to
versions[0]when no valid semver is found is pragmatic. Worth noting this behavior for non-semver version lists.
1-59: Well-structured constraint resolution pipeline.The sequential filtering approach (semver constraint → excluded versions → prereleases → select latest) is clear and maintainable. Error handling uses proper sentinel errors.
pkg/vendoring/source/provider_test.go (4)
9-81: Comprehensive provider selection tests.Good coverage of source URL formats. The table-driven approach makes it easy to extend. Consider adding a case for
git::prefix with non-GitHub sources to ensure GenericGitProvider is selected correctly.
83-110: Provider capability tests are thorough.Each provider's
SupportsOperationis tested against all known operations. Clean and verifiable.
112-190: ParseGitHubRepo tests cover key scenarios.Edge cases like module paths (
//modules/subnets) and query params (?ref=tags/1.0.0) are well covered. The error cases for invalid format and missing repo are good.
192-277: IsGitHubSource and IsGitSource tests are solid.Clear distinction between GitHub-specific and generic Git source detection. The negative cases (OCI, local paths) ensure proper boundary behavior.
pkg/vendoring/version/check_test.go (4)
10-66: ParseSemVer tests cover essential cases.Prefix stripping, prerelease identifiers, and build metadata are all tested. Good use of
require.NoErrorbefore accessing the parsed version.
68-127: FindLatestSemVerTag tests are comprehensive.The prerelease test case (lines 88-92) expects
v1.1.0-alphato be selected as latest overv1.0.5. Per semver spec,1.1.0-alpha < 1.1.0but1.1.0-alpha > 1.0.5, so this is correct behavior.
129-183: IsValidCommitSHA tests validate bounds correctly.Tests for 7-40 character range, lowercase hex requirement, and rejection of version-like strings. Matches the regex in the implementation.
185-229: ExtractGitURI tests verify normalization well.SSH URLs,
git::prefix, query params, and.gitsuffix handling are all covered. Good alignment with the implementation logic.pkg/vendoring/source/mock_provider.go (1)
1-10: Generated mock is properly configured withgo:generatedirective.The mock is correctly generated using
go.uber.org/mock/mockgen. The source file (provider.go) has the corresponding//go:generatedirective at line 3, ensuring reproducibility and compliance with project guidelines.pkg/vendoring/source/git.go (1)
24-38: LGTM.Clean delegation to version utilities. Proper perf.Track on these I/O operations.
pkg/vendoring/source/provider.go (2)
10-25: Well-structured provider interface.Clean interface design following multi-provider registry pattern. Comments are clear and properly terminated.
44-61: Clean factory implementation.Provider selection logic is appropriately ordered from most specific to fallback. Good use of dedicated detection functions.
pkg/vendoring/version/check.go (3)
27-59: Solid implementation.Good use of context timeout, proper error wrapping with static error, and correct parsing of
git ls-remoteoutput.
95-130: Good ref verification flow.The fallback chain (tag → branch → SHA format) is well-structured. The limitation around SHA validation being format-only is appropriately documented.
61-93: LGTM.
ParseSemVerandFindLatestSemVerTagare clean implementations using the Masterminds semver library.pkg/vendoring/source/github.go (6)
1-18: Clean import organization.Imports are properly grouped and aliased per project conventions.
30-39: Good constructor pattern.Factory returns the interface type, perf tracking included, timeout properly configured.
41-56: Clean delegation to version utilities.Reusing existing
ExtractGitURI,GetGitRemoteTags, andCheckGitRefavoids duplication. The underlying functions handle error wrapping.
121-131: Operation support correctly declared.The switch covers the expected operations. Matches the Provider interface contract.
168-173: Simple host detection.Works for the common cases. Edge cases like URLs containing "github.com" in a path component are unlikely in practice.
175-185: Token retrieval follows project conventions.Uses viper with
ATMOS_GITHUB_TOKENtaking precedence overGITHUB_TOKENas expected. Creating a new viper instance per call is acceptable given this function isn't on a hot path.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
errors/errors.go (2)
631-633: Consider consolidating duplicate component errors.
ErrDuplicateComponentsandErrDuplicateComponentsFoundappear to serve the same purpose. If they address distinct scenarios, clarify the difference; otherwise, unify them to reduce surface area.
637-640: Consolidate overlapping mixin errors.Three errors here may overlap:
ErrMissingMixinURI(637)ErrMixinEmpty(639)ErrMixinNotImplemented(640) duplicatesErrLocalMixinInstallationNotImplemented(28)If
ErrMixinEmptychecks for the same condition asErrMissingMixinURI, merge them. Reuse the existingErrLocalMixinInstallationNotImplementedinstead of introducingErrMixinNotImplemented.pkg/vendoring/model.go (1)
53-61: Align perf.Track labels with vendoring package and cover Init.You’ve got perf tracking in place, but labels still use the old
exec.*names, andInitis the only exported method here without tracking. To keep telemetry consistent and make searches bypkg.FuncNamereliable, I’d suggest renaming the labels tovendoring.*and adding tracking toInit:func (p pkgType) String() string { - defer perf.Track(nil, "exec.String")() + defer perf.Track(nil, "vendoring.String")() @@ func (m *modelVendor) Init() tea.Cmd { + defer perf.Track(nil, "vendoring.Init")() + if len(m.packages) == 0 { m.done = true return nil @@ func (m *modelVendor) Update(msg tea.Msg) (tea.Model, tea.Cmd) { - defer perf.Track(nil, "exec.Update")() + defer perf.Track(nil, "vendoring.Update")() @@ func (m *modelVendor) View() string { - defer perf.Track(nil, "exec.View")() + defer perf.Track(nil, "vendoring.View")() @@ func ExecuteInstall(installer pkgVendor, dryRun bool, atmosConfig *schema.AtmosConfiguration) tea.Cmd { - defer perf.Track(atmosConfig, "exec.ExecuteInstall")() + defer perf.Track(atmosConfig, "vendoring.ExecuteInstall")()As per coding guidelines, perf.Track labels should follow the
pkg.FuncNameconvention and be applied consistently to public functions.Also applies to: 194-201, 202-231, 309-339, 500-519
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
cmd/root.go(1 hunks)errors/errors.go(1 hunks)go.mod(1 hunks)pkg/schema/schema.go(1 hunks)pkg/vendoring/model.go(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- go.mod
- pkg/schema/schema.go
- cmd/root.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter)
Import organization: three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages. Maintain aliases:cfg,log,u,errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions. Usenilif no atmosConfig param
Keep files small and focused (<600 lines). One cmd/impl per file. Co-locate tests. Never use//revive:disable:file-length-limit
Use sentinel errors fromerrors/errors.gowith ErrorBuilder for all user-facing errors. Useerrors.Is()for checking errors, never string comparison.WithCause()preserves underlying error messages,WithExplanation()adds context,WithHint()provides guidance,WithContext()adds key-value pairs
Usecontext.Contextas first parameter for: cancellation signals (prop...
Files:
pkg/vendoring/model.goerrors/errors.go
pkg/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/**/*.go: Business logic packages must follow the Service-Oriented Architecture pattern with: one Service struct per domain, Provider interfaces for classes of problems, default implementations, mock implementations, and dependency injection for testability
Use interface-driven design: define interfaces for all major functionality, use dependency injection for testability, generate mocks withgo.uber.org/mock/mockgenwith//go:generatedirectives, avoid integration tests by mocking external dependencies
Create focused purpose-built packages inpkg/for new functionality, each with clear responsibility. DO NOT add new functions topkg/utils/orinternal/exec/. Each package should have focused business logic with tests
Implement registry pattern for extensibility and plugin-like architecture. Define interfaces for provider implementations, register implementations in registry, generate mocks with mockgen. Examples: Command Registry incmd/internal/registry.go, Store Registry inpkg/store/registry.go
Files:
pkg/vendoring/model.go
🧠 Learnings (30)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to pkg/**/*.go : Create focused purpose-built packages in `pkg/` for new functionality, each with clear responsibility. DO NOT add new functions to `pkg/utils/` or `internal/exec/`. Each package should have focused business logic with tests
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to {go.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to pkg/**/*.go : Implement registry pattern for extensibility and plugin-like architecture. Define interfaces for provider implementations, register implementations in registry, generate mocks with mockgen. Examples: Command Registry in `cmd/internal/registry.go`, Store Registry in `pkg/store/registry.go`
📚 Learning: 2024-10-22T23:00:20.627Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Applied to files:
pkg/vendoring/model.goerrors/errors.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to pkg/**/*.go : Create focused purpose-built packages in `pkg/` for new functionality, each with clear responsibility. DO NOT add new functions to `pkg/utils/` or `internal/exec/`. Each package should have focused business logic with tests
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : Before implementing new functionality, search `internal/exec/` and `pkg/` for existing code. Extend, don't duplicate
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2024-10-31T07:09:31.983Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:181-182
Timestamp: 2024-10-31T07:09:31.983Z
Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
pkg/vendoring/model.goerrors/errors.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : New configs support Go templating with `FuncMap()` from `internal/exec/template_funcs.go`. Implement template functions in `internal/exec/template_funcs.go`, register them, add tests, and update documentation
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to pkg/**/*.go : Use interface-driven design: define interfaces for all major functionality, use dependency injection for testability, generate mocks with `go.uber.org/mock/mockgen` with `//go:generate` directives, avoid integration tests by mocking external dependencies
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to {go.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : Import organization: three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages. Maintain aliases: `cfg`, `log`, `u`, `errUtils`
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions. Use `nil` if no atmosConfig param
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-11-30T04:16:24.155Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:34-48
Timestamp: 2025-11-30T04:16:24.155Z
Learning: In the cloudposse/atmos repository, the `defer perf.Track()` guideline applies to functions that perform meaningful work (I/O, computation, external calls), but explicitly excludes trivial accessors/mutators (e.g., simple getters, setters with single integer increments, string joins, or map appends) where the tracking overhead would exceed the actual method cost and provide no actionable performance data. Hot-path methods called in tight loops should especially avoid perf.Track() if they perform only trivial operations.
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to pkg/io/**/*.go : Use I/O layer (`pkg/io/`) for stream access (stdout/stderr/stdin), terminal capabilities, masking. Use UI layer (`pkg/ui/`) for formatting (colors, styles, markdown rendering). Never use `fmt.Fprintf(os.Stdout/Stderr, ...)`, `fmt.Println()`, or direct stream access
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-05-30T03:21:37.197Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
pkg/vendoring/model.go
📚 Learning: 2025-12-06T19:24:49.329Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.329Z
Learning: Applies to **/*.go : Use sentinel errors from `errors/errors.go` with ErrorBuilder for all user-facing errors. Use `errors.Is()` for checking errors, never string comparison. `WithCause()` preserves underlying error messages, `WithExplanation()` adds context, `WithHint()` provides guidance, `WithContext()` adds key-value pairs
Applied to files:
errors/errors.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
errors/errors.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
errors/errors.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
errors/errors.go
📚 Learning: 2025-01-25T03:51:57.689Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Applied to files:
errors/errors.go
📚 Learning: 2025-01-17T00:18:57.769Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Applied to files:
errors/errors.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
errors/errors.go
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
errors/errors.go
📚 Learning: 2024-11-22T12:38:33.132Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Applied to files:
errors/errors.go
📚 Learning: 2025-09-10T21:17:55.273Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Applied to files:
errors/errors.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
errors/errors.go
🧬 Code graph analysis (1)
pkg/vendoring/model.go (4)
errors/errors.go (1)
ErrVendorComponents(621-621)internal/exec/file_utils.go (2)
RemoveTempDir(25-32)SanitizeFileName(107-136)internal/exec/copy_glob.go (1)
CopyToTargetWithPatterns(458-499)internal/exec/oci_utils.go (1)
ProcessOciImage(34-38)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (5)
errors/errors.go (2)
594-614: Vendor update/diff errors look solid.The 20 new sentinels cover the expected failure modes for version checking, git operations, and YAML updates. Names are clear and consistent with existing patterns.
594-646: Remove unused error sentinels or add their usage.The following 8 sentinel errors are defined but never used in the codebase:
ErrInvalidGitRefErrNoUpdatesAvailableErrInvalidVersionSpecErrVersionNotFoundErrYAMLPreservationFailedErrMultipleComponentMatchesErrGitRefNotFoundErrInvalidSemanticVersionEither implement their usage in the vendor logic or remove them to keep the error definitions clean and maintainable.
⛔ Skipped due to learnings
Learnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-06T19:24:49.329Z Learning: Applies to **/*_test.go : Use `assert.ErrorIs(t, err, ErrSentinel)` for checking our own errors and stdlib errors. Only use string matching for third-party errorsLearnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-06T19:24:49.329Z Learning: Applies to **/*.go : Use sentinel errors from `errors/errors.go` with ErrorBuilder for all user-facing errors. Use `errors.Is()` for checking errors, never string comparison. `WithCause()` preserves underlying error messages, `WithExplanation()` adds context, `WithHint()` provides guidance, `WithContext()` adds key-value pairsLearnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-06T19:24:49.329Z Learning: Applies to **/*_test.go : NEVER use `assert.Contains(err.Error(), ...)` for error checking. ALWAYS use `assert.ErrorIs()` instead. NEVER use string comparison: `err.Error() == "..."` or `strings.Contains(err.Error(), ...)`Learnt from: Cerebrovinny Repo: cloudposse/atmos PR: 737 File: internal/exec/vendor_utils.go:181-182 Timestamp: 2024-10-31T07:09:31.983Z Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.Learnt from: CR Repo: cloudposse/atmos PR: 0 File: .cursor/rules/atmos-rules.mdc:0-0 Timestamp: 2025-11-24T17:35:37.209Z Learning: Applies to **/*.go : Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using `fmt.Errorf("context: %w", err)`, and consider using custom error types for domain-specific errorsLearnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-06T19:24:49.329Z Learning: Applies to **/*.go : NEVER use `--no-verify` for git commits. ALWAYS run `make lint` before committing. Hooks run go-fumpt, golangci-lint, go mod tidy. ALWAYS compile after changes: `go build . && go test ./...`. Fix errors immediatelyLearnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-06T19:24:49.329Z Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests. Make code testable using dependency injection to avoid `os.Exit`, `CheckErrorPrintAndExit`, external systems. Remove always-skipped tests - fix or delete. Table-driven tests must have real scenariosLearnt from: osterman Repo: cloudposse/atmos PR: 740 File: cmd/cmd_utils.go:340-359 Timestamp: 2024-10-23T21:36:40.262Z Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.Learnt from: samtholiya Repo: cloudposse/atmos PR: 959 File: cmd/describe_config.go:20-20 Timestamp: 2025-02-03T06:00:11.419Z Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.pkg/vendoring/model.go (3)
1-27: Vendoring package move and exec helper imports look solid.Using a dedicated
vendoringpackage and reusinginternal/exechelpers,log, andperfmatches the repo’s architecture and import/alias rules; import groups are correctly ordered and namespaced.
106-134: ErrVendorComponents usage for aggregate failures looks good.Wrapping vendor failures with
ErrVendorComponentsand including the failed count via%dkeeps the sentinel error testable while still conveying useful detail to callers.
348-372: Exec helpers for tempdirs, copy, OCI, and filename sanitization are wired correctly.
exec.RemoveTempDir,exec.CopyToTargetWithPatterns,exec.ProcessOciImage, andexec.SanitizeFileNameare used with the expected argument types and flags, with cleanup deferred correctly andsourceIsLocalFilepropagated so single‑file/local cases are handled through the shared copy logic. The Windows‑safe filename sanitization for local files viaSanitizeFileNameis a nice reuse and keeps behavior centralized.Also applies to: 374-405
| ErrComponentConfigFileNotFound = errors.New("component vendoring config file does not exist in the folder") | ||
| ErrFolderNotFound = errors.New("folder does not exist") | ||
| ErrInvalidComponentKind = errors.New("invalid 'kind' in the component vendoring config file. Supported kinds: 'ComponentVendorConfig'") | ||
| ErrUriMustSpecified = errors.New("'uri' must be specified in 'source.uri' in the component vendoring config file") |
There was a problem hiding this comment.
Fix grammatical error in sentinel name.
ErrUriMustSpecified should be ErrURIMustBeSpecified (or ErrMissingURI for brevity).
Apply this diff:
- ErrUriMustSpecified = errors.New("'uri' must be specified in 'source.uri' in the component vendoring config file")
+ ErrURIMustBeSpecified = errors.New("'uri' must be specified in 'source.uri' in the component vendoring config file")📝 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.
| ErrUriMustSpecified = errors.New("'uri' must be specified in 'source.uri' in the component vendoring config file") | |
| ErrURIMustBeSpecified = errors.New("'uri' must be specified in 'source.uri' in the component vendoring config file") |
🤖 Prompt for AI Agents
In errors/errors.go around line 645, the sentinel constant ErrUriMustSpecified
has a grammar/casing issue; rename it to ErrURIMustBeSpecified (or ErrMissingURI
if preferred) and update all references across the codebase to use the new
identifier. Change the declaration name, preserve the error string (or adjust to
match new name if you choose), run go vet/build to find references, update any
comments/tests that mention the old name, and run go fmt to apply formatting.
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Resolved merge conflict in errors/errors.go by keeping both: - Vendor update/diff errors (from feature branch) - Component path resolution errors (from main) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
what
atmos vendor updatecommand to check for and update vendored component versionsatmos vendor diffcommand to show Git diffs between component versionswhy
references
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.