fix(scaffold): preserve source in scaffold config - #2869
Conversation
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
📝 WalkthroughWalkthroughThe resolver now preserves the original source path or URI after local, hydrated, and remote scaffold resolution. Tests verify source preservation and remote ZIP loading. ChangesSource provenance
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/generator/source/resolver_test.go`:
- Around line 244-259: Update TestResolve_RemoteRecordsOriginalSource to avoid
invoking the git binary or requiring requireGit, initSourceTestGitRepo, or
runSourceTestGit. Use Go-native fixture files with a mocked fetcher, or move the
remote-source fixture setup into test-only implementation support, while
preserving the assertion that cfg.Source equals the original remote source
string.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 815c2a22-a02a-4869-b88b-6a464f49ceed
📒 Files selected for processing (2)
pkg/generator/source/resolver.gopkg/generator/source/resolver_test.go
aade2ed to
f2d9362
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2869 +/- ##
==========================================
+ Coverage 82.76% 82.77% +0.01%
==========================================
Files 1861 1861
Lines 180438 180439 +1
==========================================
+ Hits 149342 149363 +21
+ Misses 23309 23289 -20
Partials 7787 7787
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
what
atmos scaffold generate/atmos initrecording a dangling, already-deleted temp-directory path inspec.sourceof.atmos/scaffold.yamlwhenever the template source is remote (git::...or a barehttps://...URL).pkg/generator/source/resolver.go: inresolveRemote(), setconf.Source = src(the original source string the caller passed in) after loading the template configuration from the temporary download directory, instead of leavingConfiguration.Sourceas whateverLoadConfigurationFromDirwas given (the temp dir itself).pkg/generator/source/resolver_test.go: added/extended tests assertingConfiguration.Sourceholds the original source for both local and remote paths, plus a dedicated regression test (TestResolve_RemoteRecordsOriginalSource) that fails on the pre-fix code and passes after.resolveLocal), which already recorded the correct value.why
resolveRemote()downloads the template intoos.MkdirTemp("", "atmos-scaffold-"), then loaded the config with that temp dir passed in as the "source" — soConfiguration.Source, andtherefore the persisted
spec.source, ended up holding something like/var/folders/xx/.../atmos-scaffold-1234567890. That directory is removed bycleanup()immediately after the command finishes, so the recorded provenance is a dangling reference to nothing as soon as generation completes — useless for anything that might want to read it back later (e.g. a future--update/re-resolve flow), and directly contradictsSaveProjectRecord's own doc comment: "spec.source and spec.baseRef record provenance for future updates."file://...) were correct only by accident of not having a temp-dir indirection step inresolveLocal, not because anything special-cased provenance for them.atmos scaffold generate "git::https://.../scaffold-template.git" ./out --defaults, thencat ./out/.atmos/scaffold.yamlshows a/var/folders/...//tmp/...path forspec.source, and that path no longer exists on disk.references
cloudposse/atmos+spec.source/scaffold, nothing matched as of 2026-08.Summary by CodeRabbit