Skip to content

fix: base path resolution for ATMOS_BASE_PATH and --base-path with relative paths#2215

Merged
aknysh merged 14 commits intomainfrom
aknysh/fix-import-not-found
Mar 18, 2026
Merged

fix: base path resolution for ATMOS_BASE_PATH and --base-path with relative paths#2215
aknysh merged 14 commits intomainfrom
aknysh/fix-import-not-found

Conversation

@aknysh
Copy link
Copy Markdown
Member

@aknysh aknysh commented Mar 17, 2026

what

  • Fix failed to find import error when ATMOS_BASE_PATH env var (or --base-path flag / atmos_base_path provider parameter) is set to a relative path like .terraform/modules/monorepo
  • Fix failed to find import error in atmos describe affected caused by git root discovery interfering with configured base_path
  • Improve failed to find import error messages with actionable hints and context (pattern, stacks base path)
  • Formalize base path resolution semantics with 4-category classification and source-dependent anchoring

why

Since v1.202.0, resolveAbsolutePath() routes simple relative paths through git root discovery. This breaks two scenarios:

Scenario 1: terraform-provider-utils with ATMOS_BASE_PATH env var

When a user sets ATMOS_BASE_PATH=.terraform/modules/monorepo on a CI/CD worker (e.g., Spacelift), the path should resolve relative to CWD (/project/components/terraform/iam-delegated-roles/.terraform/modules/monorepo). Instead, git root discovery resolves it to /project/.terraform/modules/monorepo — a path that doesn't exist — causing failed to find import.

Scenario 2: atmos describe affected

In CI environments where the working directory structure differs from expectations, git root discovery can compute the stacks base path incorrectly, causing stack processing to fail with failed to find import.

Error message problem

The bare failed to find import error provides no context about which import failed or what path was searched, making it impossible for users to diagnose.

Fix approach

Fix 1: Source-aware base path resolution

Every base_path value is classified into one of four categories:

Category Pattern Resolution
Empty "", unset Git root → config dir → CWD (smart default)
Dot ".", "./foo", "..", "../foo" Source-dependent anchor (see below)
Bare "foo", "foo/bar", ".terraform/..." Git root search, source-independent
Absolute "/abs/path" Pass through unchanged

A BasePathSource field on AtmosConfiguration tracks whether the base path came from a runtime source or config file:

  • Runtime sources (env var ATMOS_BASE_PATH, CLI flag --base-path, provider param atmos_base_path): set BasePathSource = "runtime". Dot-prefixed paths resolve relative to CWD (shell convention).
  • Config source (base_path in atmos.yaml): dot-prefixed paths resolve relative to the directory containing atmos.yaml (config-file convention).
  • Bare paths are source-independent — they always go through git root search regardless of where they were specified.

resolveAbsolutePath() now accepts a source parameter and routes dot-prefixed paths through resolveDotPrefixPath(). The tryResolveWithGitRoot() function adds os.Stat validation — if the git-root-joined path doesn't exist but the CWD-relative path does, it falls back to the CWD-relative path.

Fix 2: Actionable error messages

Wrap ErrFailedToFindImport with the error builder pattern in FindAllStackConfigsInPathsForStack, FindAllStackConfigsInPaths, and GetGlobMatches, adding hints about checking base_path, stacks.base_path, and ATMOS_BASE_PATH. All path resolution errors use errUtils.Build(errUtils.ErrPathResolution) for consistent errors.Is() classification.

Spacelift/provider scenario

ATMOS_BASE_PATH=./.terraform/modules/monorepo (recommended dot-slash form): classified as Dot, runtime source → resolves relative to CWD. Works correctly.

ATMOS_BASE_PATH=.terraform/modules/monorepo (existing bare form): classified as Bare, goes through git root search → os.Stat at git root fails → falls back to CWD. Also works correctly.

Git root discovery compatibility

The "run Atmos from any subdirectory" feature (v1.202.0+) is not affected:

  • Source-aware resolution only changes behavior for dot-prefixed paths from runtime sources
  • Bare paths ("stacks", "foo/bar") go through the same git root search regardless of source
  • Empty base_path returns git root (unchanged)
  • Config-file dot-prefixed paths (base_path: "." in atmos.yaml) continue to resolve relative to the config directory

Verified by existing integration tests: describe_component_from_nested_dir, terraform_plan_from_nested_dir, terraform_plan_with_current_directory.

Backward compatibility

  • Dot-prefixed paths from runtime sources (ATMOS_BASE_PATH=".", --base-path=./foo): now resolve relative to CWD (shell convention). Previously resolved relative to config dir. This matches user expectations — in a shell, . means "here" (CWD)
  • Dot-prefixed paths from config file (base_path: "." in atmos.yaml): continue to resolve relative to atmos.yaml location — no change
  • Bare paths ("stacks", ".terraform/modules/monorepo"): go through git root search regardless of source, with os.Stat fallback to CWD — source-independent
  • Default/empty base_path: continues to use git root discovery (the v1.202.0 behavior)
  • ATMOS_GIT_ROOT_BASEPATH=false: continues to work as a full opt-out of git root discovery

references

  • closes Error: failed to find import #2183
  • PRD: docs/prd/base-path-resolution-semantics.md
  • Fix doc: docs/fixes/2026-03-17-failed-to-find-import-base-path-resolution.md
  • Related: terraform-provider-utils v1.32.0+ with ATMOS_BASE_PATH env var

aknysh and others added 2 commits March 17, 2026 10:11
When ATMOS_BASE_PATH env var, --base-path flag, or atmos_base_path
provider parameter provides a simple relative path (e.g.,
".terraform/modules/monorepo"), resolve it relative to CWD instead
of routing through git root discovery. This restores pre-v1.202.0
behavior for explicitly set paths while preserving git root
discovery for default/empty base paths.

Two-pronged fix:
1. configAndStacksInfo.AtmosBasePath (provider/CLI): convert to
   absolute CWD-relative immediately in InitCliConfig
2. ATMOS_BASE_PATH env var (via Viper): tryResolveWithGitRoot now
   validates the git-root-joined path exists before using it, and
   falls back to CWD-relative resolution if it doesn't

Also improves error messages for "failed to find import" using
the error builder pattern with actionable hints and context.

Fixes #2183

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verify that the os.Stat fallback in tryResolveWithGitRoot doesn't break
"run Atmos from any subdirectory" behavior. Add table-driven tests for
resolveSimpleRelativeBasePath helper. Document git root discovery
compatibility analysis with integration test evidence.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aknysh aknysh added the patch A minor, backward compatible change label Mar 17, 2026
@github-actions github-actions bot added the size/m Medium size PR label Mar 17, 2026
@aknysh aknysh self-assigned this Mar 17, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 17, 2026

Dependency Review

The following issues were found:
  • ✅ 0 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ✅ 0 package(s) with invalid SPDX license definitions
  • ⚠️ 1 package(s) with unknown licenses.
See the Details below.

License Issues

go.mod

PackageVersionLicenseIssue Type
modernc.org/sqlite1.47.0NullUnknown License
Allowed Licenses: MIT, MIT-0, Apache-2.0, BSD-2-Clause, BSD-2-Clause-Views, BSD-3-Clause, ISC, MPL-2.0, 0BSD, Unlicense, CC0-1.0, CC-BY-3.0, CC-BY-4.0, CC-BY-SA-3.0, Python-2.0, OFL-1.1, LicenseRef-scancode-generic-cla, LicenseRef-scancode-unknown-license-reference, LicenseRef-scancode-unicode, LicenseRef-scancode-google-patent-license-golang
Excluded from license check: pkg:golang/modernc.org/libc

Scanned Files

  • go.mod

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 17, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds source-aware base_path resolution: dot-prefixed values are anchored by source (runtime → CWD, config → config dir), prefer git-root for bare paths with CWD fallback, enriches glob error hints, adds tests, updates docs/PRD, and records runtime base-path source in the config schema.

Changes

Cohort / File(s) Summary
Documentation
docs/fixes/2026-03-17-failed-to-find-import-base-path-resolution.md, docs/prd/base-path-resolution-semantics.md
New incident writeup and PRD specifying classification (EMPTY/DOT/BARE/ABSOLUTE), source-aware anchoring, algorithmic resolution, migration guidance, and testability requirements.
Config initialization & resolution
pkg/config/config.go, pkg/config/config_test.go
Introduce source-aware resolveAbsolutePath/resolveDotPrefixPath, pass BasePathSource through InitCliConfig, change git-root vs CWD fallback behavior, update tests to new signature and semantics.
Utilities & glob errors
pkg/config/utils.go, pkg/utils/glob_utils.go
Wrap glob failures with richer errUtils-built errors, add user-facing hints and attach pattern/context; set BasePathSource="runtime" for env/CLI-provided base paths.
Schema
pkg/schema/schema.go
Add BasePathSource string to AtmosConfiguration (non-serialized field) to record runtime vs config origin.
Tests
pkg/config/base_path_resolution_test.go, pkg/config/config_test.go, tests/test-cases/component-path-resolution.yaml, tests/snapshots/... .golden
Add extensive base-path resolution unit tests, update existing tests and snapshots to reflect new anchoring and expectations.
Deps & NOTICE
go.mod, NOTICE
Bump several dependencies and update LICENSE URLs; no API signature changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CLI as CLI / Env / Flags
  participant Init as InitCliConfig
  participant Resolver as resolveAbsolutePath / resolveDotPrefixPath
  participant Git as GitRootDiscovery
  participant FS as FileSystem / Glob
  CLI->>Init: provide base_path (flag/env/config)
  Init->>Resolver: classify & normalize base_path (source-aware)
  alt DOT-prefixed & source=runtime
    Resolver->>FS: anchor to CWD and resolve
  else DOT-prefixed & source=config
    Resolver->>Init: anchor to config dir and resolve
  else BARE (no dot/absolute)
    Init->>Git: discover git root
    Git-->>Init: git-root path
    Init->>FS: check git-root-joined path exists
    alt exists
      FS-->>Init: use git-root path
    else not exists
      Init->>FS: check CWD-relative path
      alt exists
        FS-->>Init: use CWD-relative path
      else
        FS-->>Init: no match -> preserve git-root path for error semantics
      end
    end
  end
  Init->>FS: run glob (GetGlobMatches)
  FS-->>Init: matches or error (error enriched with hints)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • osterman
  • kevcube
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title clearly and specifically describes the main fix: handling relative paths in ATMOS_BASE_PATH and --base-path arguments by resolving them relative to CWD instead of git root, which aligns with the core changes across config resolution, testing, and error handling.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aknysh/fix-import-not-found
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/config/base_path_resolution_test.go`:
- Around line 157-188: Test TestTryResolveWithGitRoot_ExistingPathAtGitRoot only
inspects os.Stat and never invokes the path resolver; modify the test to call
the actual resolver (resolveAbsolutePath or tryResolveWithGitRoot) with the
relative "stacks" path while CWD is subDir and the git root is simulated as
tmpDir, then assert the returned absolute path equals the expected stacksDir;
ensure you capture and assert any error from resolveAbsolutePath, and restore
working directory after the test if you change it.
- Around line 207-210: The test case using a hardcoded Unix path in the test row
named "absolute path returned as-is" should be changed to compute an absolute
path at runtime to be cross-platform: replace the literal value assigned to the
test row's input with a value produced via filepath.Abs (e.g., call filepath.Abs
on a relative path or a joined path created with filepath.Join) and handle the
error if any before using it in the test, ensuring the test uses that computed
absolute path for the input field instead of the hardcoded "/usr/local/atmos".

In `@pkg/config/config.go`:
- Around line 334-351: The code currently treats all os.Stat errors the same and
returns raw errors from filepath.Abs; update the logic around gitRootJoined and
cwdJoined to distinguish "not found" vs other I/O errors by using
os.IsNotExist(err): when os.Stat(gitRootJoined) returns an error that is not
os.IsNotExist, return a wrapped/static error (use the repo's static error
pattern, e.g., fmt.Errorf("...: %w", ErrIO or similar) or existing
error-wrapping helper) instead of falling back; likewise after
filepath.Abs(path) wrap that error with the project's static error type before
returning; when os.Stat(cwdJoined) returns an error, if os.IsNotExist continue
to the final fallback, otherwise return a wrapped I/O error immediately. Ensure
you reference gitRootJoined, cwdJoined, os.Stat, and filepath.Abs in your
changes.
🪄 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: 1b226628-c26f-4b4f-91a1-5e2df566d2bc

📥 Commits

Reviewing files that changed from the base of the PR and between e0dc13b and d77a659.

📒 Files selected for processing (5)
  • docs/fixes/2026-03-17-failed-to-find-import-base-path-resolution.md
  • pkg/config/base_path_resolution_test.go
  • pkg/config/config.go
  • pkg/config/utils.go
  • pkg/utils/glob_utils.go

- Distinguish os.Stat ENOENT from permission/I/O errors in
  tryResolveWithGitRoot using os.IsNotExist() checks
- Use error builder pattern (ErrStatFile, ErrPathResolution) for
  non-ENOENT failures instead of silently falling back
- Remove WithExitCode(2) from error builders — restores default exit
  code 1, fixing helmfile apply non-existent CI test
- Replace hardcoded Unix path in test with filepath.Abs()
- Rewrite TestTryResolveWithGitRoot_ExistingPathAtGitRoot to actually
  call resolveAbsolutePath instead of just checking os.Stat preconditions
- Add TestTryResolveWithGitRoot_CWDFallback covering the core fix path
- Add TestTryResolveWithGitRoot_NeitherExists for fallback to git root

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aknysh aknysh requested a review from a team as a code owner March 17, 2026 16:14
@osterman
Copy link
Copy Markdown
Member

@aknysh please update and reconcile our PRD. I'm curious what our thinking was at the time.

https://github.com/cloudposse/atmos/blob/main/docs/prd/base-path-resolution-semantics.md

@aknysh
Copy link
Copy Markdown
Member Author

aknysh commented Mar 17, 2026

@aknysh please update and reconcile our PRD. I'm curious what our thinking was at the time.

https://github.com/cloudposse/atmos/blob/main/docs/prd/base-path-resolution-semantics.md

The PRD has a gap that directly caused this bug. The PRD covers base_path values set in atmos.yaml, but it doesn't account for the different sources of base_path:

  1. atmos.yaml — the PRD's semantics make sense here. "foo" = search git root → config dir.
  2. ATMOS_BASE_PATH env var — FR4 (line 57) says it "overrides base_path in config file" but doesn't specify resolution semantics. The PRD treats it as if the override value follows the same resolution rules as config-file values, but that's
    wrong for simple relative paths.
  3. --base-path CLI flag / atmos_base_path provider parameter — not mentioned in the PRD at all.

The core tension: The PRD says "foo" → search git root → config dir (line 28). This makes sense for config-file values (you'd write stacks in atmos.yaml meaning /stacks). But when a user sets
ATMOS_BASE_PATH=.terraform/modules/monorepo on a CI worker, they mean it relative to CWD, not git root. The PRD's design rationale (line 98) even quotes the v1.201.0 commit saying config-relative is correct for atmos.yaml, but that rationale
doesn't apply to env vars or CLI flags.

What needs reconciling:

  • FR4 needs to specify that ATMOS_BASE_PATH simple relative paths resolve relative to CWD, not git root — because env vars are set by the user in their shell context, not in a config file
  • A new FR should cover --base-path and atmos_base_path provider parameter with the same CWD-relative semantics
  • The detection logic (lines 129-141) needs a preamble: "if path was explicitly provided via env var or CLI flag, resolve simple relative paths via CWD"
  • The test cases section should include env-var and CLI-flag scenarios

My opinion: Erik is right to flag this. The PRD formalized semantics for config-file values but didn't distinguish between sources. Our fix is consistent with the PRD's spirit (config-relative for config files, CWD-relative for user-provided
overrides) but the PRD itself needs updating to codify this distinction.

Add FR6 (Runtime Source Resolution) formalizing that --base-path,
ATMOS_BASE_PATH env var, and atmos_base_path provider parameter resolve
simple relative paths relative to CWD, not git root. Document the
design rationale for source-dependent resolution, add runtime test
cases, implementation details, and Issue #2183 resolution.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aknysh aknysh requested a review from osterman March 17, 2026 16:30
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
docs/prd/base-path-resolution-semantics.md (1)

254-269: Runtime source test cases are comprehensive.

These examples match the scenarios covered in the unit tests. One minor note: the code block starting at line 256 lacks a language specifier (MD040). Not blocking.

Optional: add language specifier
-```
+```text
 # Scenario: CWD is /repo/components/terraform/vpc, git root is /repo
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/prd/base-path-resolution-semantics.md` around lines 254 - 269, The
fenced code block in the docs lacks a language specifier (MD040); update the
triple-backtick fence that starts the example (the block containing "# Scenario:
CWD is /repo/components/terraform/vpc, git root is /repo") to include a language
tag such as "text" (e.g., change ``` to ```text) so the markdown linter stops
flagging MD040 and the snippet renders with the intended plain-text formatting.
🤖 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/prd/base-path-resolution-semantics.md`:
- Around line 254-269: The fenced code block in the docs lacks a language
specifier (MD040); update the triple-backtick fence that starts the example (the
block containing "# Scenario: CWD is /repo/components/terraform/vpc, git root is
/repo") to include a language tag such as "text" (e.g., change ``` to ```text)
so the markdown linter stops flagging MD040 and the snippet renders with the
intended plain-text formatting.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5ba8a4a0-b461-462a-abad-88603c36f0ea

📥 Commits

Reviewing files that changed from the base of the PR and between d77a659 and e470b3b.

📒 Files selected for processing (4)
  • docs/prd/base-path-resolution-semantics.md
  • pkg/config/base_path_resolution_test.go
  • pkg/config/config.go
  • pkg/config/utils.go

coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 17, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 73.43750% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.88%. Comparing base (7f1f92b) to head (d432dbb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/config/config.go 63.82% 13 Missing and 4 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2215      +/-   ##
==========================================
+ Coverage   76.85%   76.88%   +0.02%     
==========================================
  Files        1001     1001              
  Lines       95361    95392      +31     
==========================================
+ Hits        73294    73346      +52     
+ Misses      17802    17782      -20     
+ Partials     4265     4264       -1     
Flag Coverage Δ
unittests 76.88% <73.43%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pkg/config/utils.go 88.05% <100.00%> (+0.90%) ⬆️
pkg/schema/schema.go 87.70% <ø> (ø)
pkg/utils/glob_utils.go 100.00% <100.00%> (ø)
pkg/config/config.go 72.03% <63.82%> (-1.03%) ⬇️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The CWDFallback test was creating a temp dir outside the git repo,
causing getGitRootOrEmpty() to return "" and skip the os.Stat fallback
logic entirely. Now creates the test dir inside the repo so git root
discovery works, increasing tryResolveWithGitRoot coverage from 76%
to 84%.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 17, 2026
aknysh and others added 2 commits March 17, 2026 18:34
Updated: anthropic-sdk-go v1.27.0, googleapis/gax-go v2.19.0,
google.golang.org/api v0.272.0, modernc.org/sqlite v1.47.0,
google.golang.org/genproto 20260316, charmbracelet/x/exp/slice
20260316.

Pinned: gocloud.dev v0.41.0 and go-fsimpl v0.3.1 (gomplate/v3
incompatible with newer versions due to s3blob.URLOpener changes).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
osterman added a commit that referenced this pull request Mar 18, 2026
…ndent semantics

Establish a unified convention where:
- Empty ("") = smart defaults (git root search)
- Dot (".", "./foo", "..", "../foo") = explicit anchor, context-dependent
  - In atmos.yaml: anchor to config directory (config-file convention)
  - In env var/CLI/provider: anchor to CWD (shell convention)
- Bare ("foo", "stacks") = search path, source-independent (always git root search)
- Absolute ("/path") = pass through unchanged

This eliminates the incongruence where "" and "." were treated identically or where
the same bare path value behaved differently based on source. The convention is now
unambiguous: empty is absence of opinion, dot is contextual anchor, bare is a search.

## Changes

**Core implementation:**
- Add `BasePathSource` field to `AtmosConfiguration` to track source (runtime vs config)
- Make `resolveAbsolutePath()` source-aware: dot-prefix resolves to CWD for runtime, config dir for config
- Mark source as "runtime" when base_path comes from env var, CLI flag, or provider parameter
- Remove `resolveSimpleRelativeBasePath()` — no longer needed with source-aware resolution
- Extract `resolveDotPrefixPath()` helper to reduce cyclomatic complexity
- Bare paths continue through git root search regardless of source

**Testing:**
- Add comprehensive tests proving dot-prefix is source-dependent, bare paths are not
- Update existing tests to reflect new convention (ATMOS_BASE_PATH=. now goes to CWD)
- All tests pass; no regressions

**Documentation:**
- Rewrite PRD with 4-category classification (Empty/Dot/Bare/Absolute)
- Add 18-row comprehensive examples table showing all scenarios
- Include source-awareness explanation and consistency proof
- Update fix documentation to reference source-aware resolution

Fixes #2183 (Tyler Rankin's ATMOS_BASE_PATH=.terraform/modules/monorepo scenario).
Fixes #1858 (empty base_path with ATMOS_CLI_CONFIG_PATH).
Stacked onto PR #2215 (aknysh/fix-import-not-found).

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
NOTICE (1)

1-1: ⚠️ Potential issue | 🔴 Critical

NOTICE file must be regenerated to pass CI.

The pipeline is failing because the NOTICE file is out of date. Run the following command locally and commit the result:

./scripts/generate-notice.sh

Based on learnings, the NOTICE file is programmatically generated and should not be manually edited.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@NOTICE` at line 1, The NOTICE file is out of date and must be regenerated to
satisfy CI; run the generate-notice.sh script to regenerate the NOTICE file and
commit the updated NOTICE (do not manually edit it). Locate the existing NOTICE
file and the generate-notice.sh script referenced in the repo, execute the
script locally to produce the new NOTICE, verify changes, and commit them so the
pipeline will pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@NOTICE`:
- Line 1: The NOTICE file is out of date and must be regenerated to satisfy CI;
run the generate-notice.sh script to regenerate the NOTICE file and commit the
updated NOTICE (do not manually edit it). Locate the existing NOTICE file and
the generate-notice.sh script referenced in the repo, execute the script locally
to produce the new NOTICE, verify changes, and commit them so the pipeline will
pass.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d3774f85-d5b2-4f49-b5ed-2e9d78bebce0

📥 Commits

Reviewing files that changed from the base of the PR and between a09e21d and a1c5902.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • NOTICE
  • go.mod

coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 18, 2026
* docs(prd): formalize base path resolution convention with source-dependent semantics

Establish a unified convention where:
- Empty ("") = smart defaults (git root search)
- Dot (".", "./foo", "..", "../foo") = explicit anchor, context-dependent
  - In atmos.yaml: anchor to config directory (config-file convention)
  - In env var/CLI/provider: anchor to CWD (shell convention)
- Bare ("foo", "stacks") = search path, source-independent (always git root search)
- Absolute ("/path") = pass through unchanged

This eliminates the incongruence where "" and "." were treated identically or where
the same bare path value behaved differently based on source. The convention is now
unambiguous: empty is absence of opinion, dot is contextual anchor, bare is a search.

## Changes

**Core implementation:**
- Add `BasePathSource` field to `AtmosConfiguration` to track source (runtime vs config)
- Make `resolveAbsolutePath()` source-aware: dot-prefix resolves to CWD for runtime, config dir for config
- Mark source as "runtime" when base_path comes from env var, CLI flag, or provider parameter
- Remove `resolveSimpleRelativeBasePath()` — no longer needed with source-aware resolution
- Extract `resolveDotPrefixPath()` helper to reduce cyclomatic complexity
- Bare paths continue through git root search regardless of source

**Testing:**
- Add comprehensive tests proving dot-prefix is source-dependent, bare paths are not
- Update existing tests to reflect new convention (ATMOS_BASE_PATH=. now goes to CWD)
- All tests pass; no regressions

**Documentation:**
- Rewrite PRD with 4-category classification (Empty/Dot/Bare/Absolute)
- Add 18-row comprehensive examples table showing all scenarios
- Include source-awareness explanation and consistency proof
- Update fix documentation to reference source-aware resolution

Fixes #2183 (Tyler Rankin's ATMOS_BASE_PATH=.terraform/modules/monorepo scenario).
Fixes #1858 (empty base_path with ATMOS_CLI_CONFIG_PATH).
Stacked onto PR #2215 (aknysh/fix-import-not-found).

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix: update tests for source-aware base path resolution

Update CLI test cases to use correct relative paths (e.g., "../..") instead
of "." for ATMOS_BASE_PATH env var, since env vars are now "runtime" source
and "." resolves to CWD (shell convention) not config dir.

Add unit tests for source-aware resolution: dot-prefix fallback, absolute
pass-through, bare path without git root, and BasePathSource tracking.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: use real absolute path in test for Windows compatibility

Use t.TempDir() instead of constructing path from filepath.Separator,
which lacks a drive letter on Windows and isn't recognized as absolute.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
Co-authored-by: aknysh <andriy.knysh@gmail.com>
@github-actions github-actions bot added size/l Large size PR and removed size/m Medium size PR labels Mar 18, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pkg/config/config.go (1)

255-283: 🛠️ Refactor suggestion | 🟠 Major

Wrap the new dot-path failures with the standard sentinels.

resolveDotPrefixPath() returns raw fmt.Errorf values on every failure branch, so callers lose errors.Is classification for path-resolution failures that the new tryResolveWithGitRoot() branches now preserve. Please route these through errUtils.ErrPathResolution / the builder pattern as well.

As per coding guidelines, "All errors must be wrapped using static errors defined in errors/errors.go, use errors.Join for combining multiple errors, use fmt.Errorf with %w for adding context, use error builder for complex errors, use errors.Is() for checking - never use dynamic errors directly."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/config/config.go` around lines 255 - 283, resolveDotPrefixPath currently
returns raw fmt.Errorf errors which prevents callers and tryResolveWithGitRoot
from matching path-resolution failures; update every error return in
resolveDotPrefixPath to wrap the underlying error with the standard sentinel
errUtils.ErrPathResolution (from errors/errors.go) using the prescribed
builder/pattern (or errors.Join/%w as appropriate) so callers can use errors.Is
to detect path-resolution failures—ensure each branch (runtime, config with
cliConfigPath, and fallback) constructs the new error by combining context (the
fmt message) with errUtils.ErrPathResolution and the original err, mirroring how
tryResolveWithGitRoot produces its wrapped errors.
🤖 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-17-failed-to-find-import-base-path-resolution.md`:
- Around line 126-139: The docs describe pre-normalizing a bare ATMOS_BASE_PATH
to an absolute path in InitCliConfig but the code in pkg/config/config.go does
not implement that behavior; update InitCliConfig (after calling
processAtmosConfigs()) to check configAndStacksInfo.AtmosBasePath first, then
fallback to os.Getenv("ATMOS_BASE_PATH"), and if the chosen path is a "simple
relative" path (does not start with ".", "..", "./", or "../") convert it to an
absolute path using filepath.Abs() while leaving paths that start with "." or
".." to be resolved later by resolveAbsolutePath(); ensure you reference these
symbols (InitCliConfig, processAtmosConfigs, configAndStacksInfo.AtmosBasePath,
os.Getenv("ATMOS_BASE_PATH"), filepath.Abs, resolveAbsolutePath) so tests
expecting ATMOS_BASE_PATH="." to resolve to CWD remain correct.

In `@pkg/config/config.go`:
- Around line 250-252: The current branch always calls
tryResolveWithGitRoot(path, isExplicitRelative, cliConfigPath) for bare relative
paths which causes runtime-sourced paths to prefer the git-root copy even when
the user supplied an explicit runtime base override; update the branch handling
where source == "runtime" and path is a bare relative (e.g., in the function
that currently returns tryResolveWithGitRoot) to first detect explicit runtime
overrides (CLI flags like --base-path or env vars
ATMOS_BASE_PATH/atmos_base_path represented by cliConfigPath/related config),
and if an explicit runtime base is present prefer resolving against the CWD (use
the existing tryResolveWithCwd or equivalent resolution logic) before falling
back to tryResolveWithGitRoot; ensure you reference and use the existing
parameters path, isExplicitRelative, source and cliConfigPath so behavior only
changes for runtime-sourced, bare relative paths with an explicit runtime base.

---

Outside diff comments:
In `@pkg/config/config.go`:
- Around line 255-283: resolveDotPrefixPath currently returns raw fmt.Errorf
errors which prevents callers and tryResolveWithGitRoot from matching
path-resolution failures; update every error return in resolveDotPrefixPath to
wrap the underlying error with the standard sentinel errUtils.ErrPathResolution
(from errors/errors.go) using the prescribed builder/pattern (or errors.Join/%w
as appropriate) so callers can use errors.Is to detect path-resolution
failures—ensure each branch (runtime, config with cliConfigPath, and fallback)
constructs the new error by combining context (the fmt message) with
errUtils.ErrPathResolution and the original err, mirroring how
tryResolveWithGitRoot produces its wrapped errors.
🪄 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: 314624d8-4870-4216-b5c9-13ba247db74c

📥 Commits

Reviewing files that changed from the base of the PR and between a1c5902 and f8a6044.

📒 Files selected for processing (9)
  • docs/fixes/2026-03-17-failed-to-find-import-base-path-resolution.md
  • docs/prd/base-path-resolution-semantics.md
  • pkg/config/base_path_resolution_test.go
  • pkg/config/config.go
  • pkg/config/config_test.go
  • pkg/config/utils.go
  • pkg/schema/schema.go
  • tests/snapshots/TestCLICommands_describe_component_with_relative_path.stdout.golden
  • tests/test-cases/component-path-resolution.yaml
✅ Files skipped from review due to trivial changes (1)
  • tests/snapshots/TestCLICommands_describe_component_with_relative_path.stdout.golden

Replace raw fmt.Errorf calls in resolveDotPrefixPath, tryResolveWithGitRoot,
and tryResolveWithConfigPath with errUtils.Build(errUtils.ErrPathResolution)
for consistent error classification via errors.Is().

Update fix doc to reflect source-aware resolution with BasePathSource
tracking instead of the old pre-normalize approach.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aknysh aknysh changed the title fix: resolve explicit base paths relative to CWD, not git root fix: source-aware base path resolution for explicit paths Mar 18, 2026
@aknysh aknysh changed the title fix: source-aware base path resolution for explicit paths fix: base path resolution for ATMOS_BASE_PATH and --base-path with relative paths Mar 18, 2026
Extract absPathOrError() helper to consolidate 7 identical filepath.Abs
error builder patterns into one function. Remove dead isExplicitRelative
parameter and code block from tryResolveWithGitRoot (unreachable since
dot-prefixed paths are routed to resolveDotPrefixPath before reaching it).

resolveDotPrefixPath: 80% → 100%, tryResolveWithConfigPath: 80% → 100%.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Restore the simple if/else pseudocode block (per review feedback) and add
a comprehensive quick-reference showing resolution examples for config-file
source, runtime source, and no-git-repo scenarios. Both blocks validated
against the implementation in pkg/config/config.go.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aknysh aknysh requested a review from osterman March 18, 2026 16:42
@aknysh aknysh merged commit bdd2758 into main Mar 18, 2026
58 checks passed
@aknysh aknysh deleted the aknysh/fix-import-not-found branch March 18, 2026 21:29
@github-actions
Copy link
Copy Markdown

These changes were released in v1.210.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch A minor, backward compatible change size/l Large size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error: failed to find import

2 participants