Skip to content

fix: Correct base path resolution semantics and fallback order#1872

Merged
aknysh merged 6 commits intoaknysh/fix-path-based-component-resolutionfrom
osterman/base-path-semantics
Dec 15, 2025
Merged

fix: Correct base path resolution semantics and fallback order#1872
aknysh merged 6 commits intoaknysh/fix-path-based-component-resolutionfrom
osterman/base-path-semantics

Conversation

@osterman
Copy link
Copy Markdown
Member

@osterman osterman commented Dec 14, 2025

what

  • Fixes path resolution to treat "." and ".." as config-file-relative (following the convention of tsconfig.json, package.json, etc.)
  • Corrects the fallback order for path resolution: config dir → git root → CWD (last resort)
  • Adds !cwd YAML function for explicit CWD-relative paths when needed
  • Renames test scenarios from issue numbers to behavior-based descriptions
  • Updates documentation with comprehensive path resolution semantics table
  • Moves test fixtures to use descriptive names instead of issue numbers

why

Issue #1858 revealed that empty base_path was not properly triggering git root discovery when atmos.yaml is in a deeply nested subdirectory. The fix implements correct path resolution semantics where config-file-relative paths (. and ..) work like other configuration files, and smart defaults (empty string) trigger git root discovery with proper fallback order.

references

Closes #1858

@osterman @claude
fix: Implement correct base path resolution semantics with proper fal…
a575478 
…lback order

- Fix path resolution to treat "." and ".." as config-file-relative (not CWD-relative)
  following the convention of tsconfig.json, package.json, and other config files
- Correct the fallback order: config dir → git root → CWD (last resort)
  Previously CWD was an immediate fallback, now it's only used if git root is unavailable
- Add !cwd YAML function for explicit CWD-relative paths when needed
- Rename test scenarios from issue numbers to behavior descriptions
- Add comprehensive path resolution tests covering edge cases and fallback scenarios
- Update documentation with path resolution semantics table
- Move fixture from issue-1858 → nested-config-empty-base-path (descriptive naming)

Closes #1858

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@osterman osterman requested review from a team as code owners December 14, 2025 04:19
@github-actions github-actions bot added the size/xl Extra large size PR label Dec 14, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 14, 2025 •
edited
Loading

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

@mergify
Copy link
Copy Markdown

mergify bot commented Dec 14, 2025

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.
Consider refactoring it into smaller, more focused PRs to facilitate a smoother review process.

@mergify mergify bot added the stacked Stacked label Dec 14, 2025
@mergify mergify bot mentioned this pull request Dec 14, 2025
Closed
@osterman @claude
docs: Add config file search order to PRD and website docs
6273b5e 
Document the atmos.yaml discovery order in both:
- docs/prd/base-path-resolution-semantics.md (FR5)
- website/docs/cli/configuration/configuration.mdx

Search order (highest to lowest priority):
1. CLI flags (--config, --config-path)
2. Environment variable (ATMOS_CLI_CONFIG_PATH)
3. Current directory (./atmos.yaml) - CWD only
4. Git repository root (repo-root/atmos.yaml)
5. Parent directory search
6. Home directory (~/.atmos/atmos.yaml)
7. System directory (/usr/local/etc/atmos/atmos.yaml)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@osterman @claude
fix: Isolate tests from git root discovery
d14d9ec 
Update tests that use t.Chdir() to fixture directories to set
ATMOS_CLI_CONFIG_PATH, which:
- Isolates tests from the repo's root atmos.yaml
- Disables parent directory search (per pkg/config/load.go)
- Ensures fixtures' atmos.yaml is used, not the repo's

Also adds !cwd to the expected tags list in yaml_utils_test.go
to account for the new YAML function.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@osterman @claude
fix: Use Trace level for config discovery logs to avoid snapshot fail…
441e4ca 
…ures

Changed log.Debug() calls to log.Trace() for config discovery messages
to prevent them from appearing in test output and causing golden file
mismatches. Added constant for ATMOS_CLI_CONFIG_PATH env var to satisfy
revive linter.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@osterman @claude
fix: Update golden snapshots for log level change
d732147 
Updated golden snapshot files to reflect the log level change from
Debug to Trace for config discovery messages:
- Removed "Found config ENV ATMOS_CLI_CONFIG_PATH=..." lines from
  Debug-level test snapshots (now at Trace level)
- Updated Trace-level test snapshots to show "TRCE  Found config ENV..."
  instead of "DEBU  Found config ENV..."
- Added new "TRCE  Found atmos.yaml in current directory" lines to
  Trace-level test snapshots

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@osterman @claude
fix: Update golden snapshot for atmos_describe_config_imports
aa20770 
Removed "Found config ENV ATMOS_CLI_CONFIG_PATH=..." lines since they're
now at Trace level and won't appear in Debug output.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@aknysh aknysh merged commit a899f83 into aknysh/fix-path-based-component-resolution Dec 15, 2025
45 of 49 checks passed
@aknysh aknysh deleted the osterman/base-path-semantics branch December 15, 2025 18:05
aknysh added a commit that referenced this pull request Dec 18, 2025
@aknysh @claude
@autofix-ci @osterman
fix: Fix base path resolution and fallback order (#1868)
54f9778 
* updates

* updates

* updates

* docs: Add manual testing section for path resolution fix

Added "Manual Testing" section to path-resolution-regression.md with:
- Fixture directory structure explanation
- Step-by-step CLI commands to verify the fix
- Comparison table showing expected vs broken behavior
- Instructions for using ATMOS_CLI_CONFIG_PATH with the test fixture

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* updates

* [autofix.ci] apply automated fixes

* fix: Treat single dot (.) as explicit relative path in path resolution

Updated resolveAbsolutePath() to treat a single dot "." as an explicit
relative path that resolves relative to the CLI config directory
(where atmos.yaml is located), rather than resolving to CWD.

This fixes the component-path-resolution tests that use
ATMOS_BASE_PATH="." with ATMOS_CLI_CONFIG_PATH pointing to the repo root.

The path resolution logic now:
1. Absolute paths → return as-is
2. Paths starting with "..", "./" or exactly "." → resolve relative to
   cliConfigPath (atmos.yaml location)
3. Simple paths like "stacks", "components/terraform", empty string →
   resolve relative to CWD (backward compatibility for issue #1858)

This maintains backward compatibility for issue #1858 while also
supporting the path-based component resolution feature.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* add tests

* fix: Tighten explicit relative path detection and make tests platform-neutral

Addresses CodeRabbit review comments:

1. Tighten "explicit relative" path detection:
   - Use exact matches for "." and ".." to avoid misclassifying
     paths like ".hidden" or "..foo" as explicit relative paths
   - Use separator-aware prefix checks (filepath.Separator) for
     "./" and "../" patterns
   - Allow forward slashes on Windows for portability in configs

2. Make absolute path tests platform-neutral:
   - Replace hardcoded Unix paths ("/absolute/path") with paths
     derived from t.TempDir() using filepath.Join()
   - Added test case for "..foo" edge case

3. Updated documentation to reflect the tighter detection logic
   and cross-platform handling.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Correct version mismatch in TestManager_Rebuild test

The test was setting ATMOS_VERSION to "1.202.0" in the config but
expecting "1.201.0" in the assertion. This was likely an oversight
when the version was bumped - line 412 was updated but line 434
was not.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* address comments, add tests

* fix: Implement correct base path resolution semantics with proper fallback order

- Fix path resolution to treat "." and ".." as config-file-relative (not CWD-relative)
  following the convention of tsconfig.json, package.json, and other config files
- Correct the fallback order: config dir → git root → CWD (last resort)
  Previously CWD was an immediate fallback, now it's only used if git root is unavailable
- Add !cwd YAML function for explicit CWD-relative paths when needed
- Rename test scenarios from issue numbers to behavior descriptions
- Add comprehensive path resolution tests covering edge cases and fallback scenarios
- Update documentation with path resolution semantics table
- Move fixture from issue-1858 → nested-config-empty-base-path (descriptive naming)

Closes #1858

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* docs: Add config file search order to PRD and website docs

Document the atmos.yaml discovery order in both:
- docs/prd/base-path-resolution-semantics.md (FR5)
- website/docs/cli/configuration/configuration.mdx

Search order (highest to lowest priority):
1. CLI flags (--config, --config-path)
2. Environment variable (ATMOS_CLI_CONFIG_PATH)
3. Current directory (./atmos.yaml) - CWD only
4. Git repository root (repo-root/atmos.yaml)
5. Parent directory search
6. Home directory (~/.atmos/atmos.yaml)
7. System directory (/usr/local/etc/atmos/atmos.yaml)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Isolate tests from git root discovery

Update tests that use t.Chdir() to fixture directories to set
ATMOS_CLI_CONFIG_PATH, which:
- Isolates tests from the repo's root atmos.yaml
- Disables parent directory search (per pkg/config/load.go)
- Ensures fixtures' atmos.yaml is used, not the repo's

Also adds !cwd to the expected tags list in yaml_utils_test.go
to account for the new YAML function.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Use Trace level for config discovery logs to avoid snapshot failures

Changed log.Debug() calls to log.Trace() for config discovery messages
to prevent them from appearing in test output and causing golden file
mismatches. Added constant for ATMOS_CLI_CONFIG_PATH env var to satisfy
revive linter.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Update golden snapshots for log level change

Updated golden snapshot files to reflect the log level change from
Debug to Trace for config discovery messages:
- Removed "Found config ENV ATMOS_CLI_CONFIG_PATH=..." lines from
  Debug-level test snapshots (now at Trace level)
- Updated Trace-level test snapshots to show "TRCE  Found config ENV..."
  instead of "DEBU  Found config ENV..."
- Added new "TRCE  Found atmos.yaml in current directory" lines to
  Trace-level test snapshots

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Update golden snapshot for atmos_describe_config_imports

Removed "Found config ENV ATMOS_CLI_CONFIG_PATH=..." lines since they're
now at Trace level and won't appear in Debug output.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Correct base path resolution semantics and fallback order (#1872)

* fix: Implement correct base path resolution semantics with proper fallback order

- Fix path resolution to treat "." and ".." as config-file-relative (not CWD-relative)
  following the convention of tsconfig.json, package.json, and other config files
- Correct the fallback order: config dir → git root → CWD (last resort)
  Previously CWD was an immediate fallback, now it's only used if git root is unavailable
- Add !cwd YAML function for explicit CWD-relative paths when needed
- Rename test scenarios from issue numbers to behavior descriptions
- Add comprehensive path resolution tests covering edge cases and fallback scenarios
- Update documentation with path resolution semantics table
- Move fixture from issue-1858 → nested-config-empty-base-path (descriptive naming)

Closes #1858

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* docs: Add config file search order to PRD and website docs

Document the atmos.yaml discovery order in both:
- docs/prd/base-path-resolution-semantics.md (FR5)
- website/docs/cli/configuration/configuration.mdx

Search order (highest to lowest priority):
1. CLI flags (--config, --config-path)
2. Environment variable (ATMOS_CLI_CONFIG_PATH)
3. Current directory (./atmos.yaml) - CWD only
4. Git repository root (repo-root/atmos.yaml)
5. Parent directory search
6. Home directory (~/.atmos/atmos.yaml)
7. System directory (/usr/local/etc/atmos/atmos.yaml)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Isolate tests from git root discovery

Update tests that use t.Chdir() to fixture directories to set
ATMOS_CLI_CONFIG_PATH, which:
- Isolates tests from the repo's root atmos.yaml
- Disables parent directory search (per pkg/config/load.go)
- Ensures fixtures' atmos.yaml is used, not the repo's

Also adds !cwd to the expected tags list in yaml_utils_test.go
to account for the new YAML function.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Use Trace level for config discovery logs to avoid snapshot failures

Changed log.Debug() calls to log.Trace() for config discovery messages
to prevent them from appearing in test output and causing golden file
mismatches. Added constant for ATMOS_CLI_CONFIG_PATH env var to satisfy
revive linter.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Update golden snapshots for log level change

Updated golden snapshot files to reflect the log level change from
Debug to Trace for config discovery messages:
- Removed "Found config ENV ATMOS_CLI_CONFIG_PATH=..." lines from
  Debug-level test snapshots (now at Trace level)
- Updated Trace-level test snapshots to show "TRCE  Found config ENV..."
  instead of "DEBU  Found config ENV..."
- Added new "TRCE  Found atmos.yaml in current directory" lines to
  Trace-level test snapshots

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Update golden snapshot for atmos_describe_config_imports

Removed "Found config ENV ATMOS_CLI_CONFIG_PATH=..." lines since they're
now at Trace level and won't appear in Debug output.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

---------

Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>

* address comments, add tests

* address comments, add tests

* address comments, add tests

* test: Add ATMOS_CLI_CONFIG_PATH isolation to fix Windows CI timeouts

Tests in internal/exec/ that use t.Chdir() without setting
ATMOS_CLI_CONFIG_PATH trigger expensive git root discovery and
parent directory search operations on every InitCliConfig() call.

On Windows, these git operations are significantly slower, causing
the test suite to timeout after 30 minutes.

This fix adds t.Setenv("ATMOS_CLI_CONFIG_PATH", ".") after each
t.Chdir() call to:
- Disable parent directory search for atmos.yaml
- Disable git root discovery
- Isolate tests from the repository's atmos.yaml

Files modified:
- component_path_resolution_test.go
- describe_affected_authmanager_test.go
- describe_component_auth_override_test.go
- describe_component_authmanager_test.go
- describe_component_provenance_test.go
- describe_component_test.go
- describe_dependents_authmanager_propagation_test.go
- describe_dependents_test.go
- describe_stacks_authmanager_propagation_test.go
- describe_stacks_test.go
- validate_component_test.go
- yaml_func_template_test.go

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix: Prevent kubeconfig_path inheritance in demo-helmfile example

The git root discovery feature (added in #1773) causes the repo root's
atmos.yaml configuration to be inherited by examples. This resulted in
the demo-helmfile example inheriting kubeconfig_path: "/dev/shm" from
the repo root, which overwrote the KUBECONFIG env var set by the CI
workflow.

The fix explicitly sets kubeconfig_path: "" in the demo-helmfile's
atmos.yaml to prevent this inheritance, allowing the CI-provided
KUBECONFIG env var to be used.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* address comments, add tests

* fix Windows tests

* address comments

* fix Windows tests

* update deps

* [autofix.ci] apply automated fixes

* updates

* [autofix.ci] apply automated fixes

* updates

* address comments, add tests

* address comments, add tests

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Erik Osterman <erik@cloudposse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aknysh aknysh aknysh approved these changes

Assignees

No one assigned

Labels

size/xl Extra large size PR stacked Stacked

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@osterman @aknysh