fix: base path resolution fallback when git root is unavailable#2236
Merged
fix: base path resolution fallback when git root is unavailable#2236
Conversation
Make tryResolveWithGitRoot and tryResolveWithConfigPath source-aware by passing the source parameter through the call chain. For runtime sources (ATMOS_BASE_PATH env var, --base-path flag, atmos_base_path provider param), tryResolveWithConfigPath now tries CWD first before config dir. Both paths use os.Stat validation with fallback. This fixes "failed to find import" on CI workers without .git where the v1.210.1 os.Stat fallback in tryResolveWithGitRoot was bypassed because getGitRootOrEmpty returned empty. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2236 +/- ##
==========================================
+ Coverage 76.89% 76.92% +0.02%
==========================================
Files 1003 1003
Lines 95524 95545 +21
==========================================
+ Hits 73456 73495 +39
+ Misses 17805 17783 -22
- Partials 4263 4267 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
📝 WalkthroughWalkthroughMakes base-path resolution source-aware to fix a CI-only Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as InitCliConfig
participant Resolve as resolveAbsolutePath
participant GitRoot as tryResolveWithGitRoot
participant ConfigPath as tryResolveWithConfigPath
participant CWD as tryCWDRelative
participant Disk as os.Stat
Caller->>Resolve: path, source (runtime/config)
Resolve->>GitRoot: attempt git-root resolution
GitRoot->>Disk: stat candidate under git root
alt exists
GitRoot-->>Resolve: return git-root path
else
GitRoot->>ConfigPath: delegate to config-path resolution (with source)
ConfigPath->>Disk: stat config-dir candidate
alt config exists
ConfigPath-->>Resolve: return config-dir path
else
ConfigPath->>CWD: try CWD-relative (order depends on source)
CWD->>Disk: stat CWD candidate
alt CWD exists
CWD-->>Resolve: return CWD path
else
CWD-->>Resolve: error
end
end
end
Resolve-->>Caller: final path or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/config/config.go (1)
387-398: Consider distinguishing stat error types.The helper swallows all
os.Staterrors uniformly. UnliketryResolveWithGitRoot(lines 332-337), this doesn't distinguish between "not exist" and permission/access errors. For a fallback helper this is acceptable, but permission errors could silently fall through to the next candidate.If you want parity with the git-root path:
♻️ Optional: surface non-NotExist errors
func tryCWDRelative(path string) (string, bool) { cwdJoined, err := absPathOrError(path, fmt.Sprintf(cwdResolutionErrFmt, path)) if err != nil { return "", false } - if _, statErr := os.Stat(cwdJoined); statErr == nil { + if info, statErr := os.Stat(cwdJoined); statErr == nil { + _ = info // exists log.Trace("Path resolved relative to CWD", "path", path, "resolved", cwdJoined) return cwdJoined, true + } else if !os.IsNotExist(statErr) { + log.Trace("Permission or access error checking CWD path", "path", cwdJoined, "error", statErr) } return "", false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/config/config.go` around lines 387 - 398, The tryCWDRelative helper currently swallows all os.Stat errors; update tryCWDRelative to mirror tryResolveWithGitRoot by checking os.IsNotExist(statErr) and treating "not exist" as the expected fallback, but for any other statErr surface it (e.g., log.Warn or log.Error with the error value and context) instead of silently ignoring it; keep absPathOrError usage for the initial resolution and reference the tryCWDRelative and tryResolveWithGitRoot functions when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/fixes/2026-03-19-failed-to-find-import-no-git-root-fallback.md`:
- Around line 398-400: The references section incorrectly states current
behavior for tryResolveWithConfigPath; reword the Line 399 note to indicate
pre-fix behavior (e.g., "tryResolveWithConfigPath — pre-fix: lacked os.Stat
fallback") so it doesn't conflict with the implemented fix described above;
ensure tryResolveWithGitRoot and getGitRootOrEmpty remain described accurately
and that the wording clearly distinguishes pre-fix vs implemented behavior for
tryResolveWithConfigPath.
In `@internal/exec/stack_processor_utils_test.go`:
- Around line 491-537: The test fixture allComponentsMap currently models two
isolated pairs (iam-delegated-roles-defaults/iam-delegated-roles and
eks-defaults/eks); add a cross-dependency so the test exercises coupled paths —
for example, add the other component’s defaults into one real component’s
metadata.inherits array (e.g., add "eks-defaults" to
iam-delegated-roles.metadata.inherits or add "iam-delegated-roles-defaults" to
eks.metadata.inherits) and adjust any vars if needed to reflect the dependency;
update only the inherits arrays on iam-delegated-roles or eks in
allComponentsMap so mergeComponentConfigurations and the dependency traversal
see the cross-link.
---
Nitpick comments:
In `@pkg/config/config.go`:
- Around line 387-398: The tryCWDRelative helper currently swallows all os.Stat
errors; update tryCWDRelative to mirror tryResolveWithGitRoot by checking
os.IsNotExist(statErr) and treating "not exist" as the expected fallback, but
for any other statErr surface it (e.g., log.Warn or log.Error with the error
value and context) instead of silently ignoring it; keep absPathOrError usage
for the initial resolution and reference the tryCWDRelative and
tryResolveWithGitRoot functions when making this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e4a70eff-f627-4d04-90d7-975824dce0f4
📒 Files selected for processing (6)
docs/fixes/2026-03-19-failed-to-find-import-no-git-root-fallback.mdinternal/exec/stack_processor_utils_test.gopkg/config/base_path_resolution_test.gopkg/config/config.gopkg/config/config_test.gopkg/config/utils.go
- Clarify pre-fix vs post-fix wording in fix doc references section - Add cross-dependency to multiple abstract components cycle test - Log permission/access errors in tryCWDRelative for parity with tryResolveWithGitRoot Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/fixes/2026-03-19-failed-to-find-import-no-git-root-fallback.md (1)
186-223: Consider noting the constant usage.The pseudo-code uses the literal string
"runtime"(Line 188), but the actual implementation uses a constantbasePathSourceRuntime. This is fine for documentation clarity, but a brief note like "// where 'runtime' is basePathSourceRuntime constant" could help readers cross-reference the actual code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/fixes/2026-03-19-failed-to-find-import-no-git-root-fallback.md` around lines 186 - 223, The doc example uses the literal "runtime" but the real code uses the constant basePathSourceRuntime; update the comment around the conditional in tryResolveWithConfigPath to mention that "runtime" corresponds to the basePathSourceRuntime constant (e.g., add a brief inline note like "// 'runtime' == basePathSourceRuntime constant") so readers can cross-reference the actual implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/fixes/2026-03-19-failed-to-find-import-no-git-root-fallback.md`:
- Around line 186-223: The doc example uses the literal "runtime" but the real
code uses the constant basePathSourceRuntime; update the comment around the
conditional in tryResolveWithConfigPath to mention that "runtime" corresponds to
the basePathSourceRuntime constant (e.g., add a brief inline note like "//
'runtime' == basePathSourceRuntime constant") so readers can cross-reference the
actual implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 654157f8-c783-4f18-836c-fa2229f1841a
📒 Files selected for processing (3)
docs/fixes/2026-03-19-failed-to-find-import-no-git-root-fallback.mdinternal/exec/stack_processor_utils_test.gopkg/config/config.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/config/config.go
osterman
approved these changes
Mar 20, 2026
|
These changes were released in v1.211.0-rc.1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
failed to find importerror whenATMOS_BASE_PATHis set to a relative path on CI workers without a.gitdirectory (e.g., Spacelift)tryResolveWithGitRootandtryResolveWithConfigPathsource-aware by passing thesourceparameter through the call chainATMOS_BASE_PATHenv var,--base-pathflag,atmos_base_pathprovider param),tryResolveWithConfigPathnow tries CWD first before config diros.Statvalidation with fallbackmetadata.componentstack overflow fix verificationbasePathSourceRuntimeandcwdResolutionErrFmtconstantstryCWDRelativehelper to reduce cognitive complexitywhy
The v1.210.1 fix (PR #2215) added
os.Stat+ CWD fallback totryResolveWithGitRoot, but this only works when git root IS available. On CI workers without.git(e.g., Spacelift),getGitRootOrEmpty()returns""and the code falls through totryResolveWithConfigPath— which lacked the same fallback. It unconditionally joined withcliConfigPath(theatmos.yamldirectory), producing the wrong path.The broken code path (before this fix)
ATMOS_BASE_PATH=.terraform/modules/monorepoprocessEnvVarssetsBasePathandBasePathSource = "runtime"resolveAbsolutePathclassifies.terraform/modules/monorepoas a bare path → callstryResolveWithGitRootgetGitRootOrEmpty()returns""— no.giton CItryResolveWithConfigPath(".terraform/modules/monorepo", "/workspace")/workspace/.terraform/modules/monorepo— WRONG (doesn't exist)/workspace/components/terraform/iam-delegated-roles/.terraform/modules/monorepoWhy absolute paths work
When
ATMOS_BASE_PATHis set to an absolute path,resolveAbsolutePathreturns it as-is at the first check (filepath.IsAbs), bypassing all resolution logic.The asymmetry fixed
tryResolveWithGitRootos.Stat+ CWD fallback (v1.210.1)sourceto fallbacktryResolveWithConfigPathos.Stat, no CWD fallbackos.StatResolution order after fix
base_pathinatmos.yaml)Stack overflow verification
The same user also reported
fatal error: stack overflowwhen abstract components havemetadata.componentset (fixed in v1.210.0, PR #2214). Additional tests were written to verify the cycle detection works for various patterns including multiple abstract/real pairs, cross-component cycles, anddefer deletere-entry. All pass — the cycle detection is working correctly for all reproducible patterns.references
metadata.component, v1.210.0)docs/fixes/2026-03-19-failed-to-find-import-no-git-root-fallback.mddocs/fixes/2026-03-17-failed-to-find-import-base-path-resolution.mddocs/fixes/2026-03-16-metadata-component-abstract-stack-overflow.mdSummary by CodeRabbit
Documentation
Bug Fixes
Tests