Skip to content

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

Closed
osterman wants to merge 13 commits intomainfrom
osterman/base-path-semantics
Closed

fix: Correct base path resolution semantics and fallback order#1871
osterman wants to merge 13 commits intomainfrom
osterman/base-path-semantics

Conversation

@osterman
Copy link
Copy Markdown
Member

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

aknysh and others added 13 commits December 10, 2025 21:31
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>
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>
…-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>
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>
…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:15
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 14, 2025

Warning

Rate limit exceeded

@osterman has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 20 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 638d1b8 and a575478.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (23)
  • NOTICE (16 hunks)
  • docs/fixes/path-resolution-regression.md (1 hunks)
  • docs/prd/base-path-resolution-semantics.md (1 hunks)
  • examples/quick-start-advanced/Dockerfile (1 hunks)
  • go.mod (7 hunks)
  • pkg/config/config.go (2 hunks)
  • pkg/config/config_test.go (1 hunks)
  • pkg/config/git_root.go (2 hunks)
  • pkg/config/load.go (4 hunks)
  • pkg/config/process_yaml.go (7 hunks)
  • pkg/config/process_yaml_test.go (2 hunks)
  • pkg/devcontainer/lifecycle_rebuild_test.go (2 hunks)
  • pkg/utils/git.go (1 hunks)
  • pkg/utils/yaml_utils.go (3 hunks)
  • tests/fixtures/scenarios/cli-config-path/components/terraform/test-component/main.tf (1 hunks)
  • tests/fixtures/scenarios/cli-config-path/config/atmos.yaml (1 hunks)
  • tests/fixtures/scenarios/cli-config-path/stacks/dev.yaml (1 hunks)
  • tests/fixtures/scenarios/nested-config-empty-base-path/components/terraform/test-component/main.tf (1 hunks)
  • tests/fixtures/scenarios/nested-config-empty-base-path/rootfs/usr/local/etc/atmos/atmos.yaml (1 hunks)
  • tests/fixtures/scenarios/nested-config-empty-base-path/stacks/dev.yaml (1 hunks)
  • website/docs/cli/configuration/configuration.mdx (2 hunks)
  • website/docs/functions/yaml/cwd.mdx (1 hunks)
  • website/docs/functions/yaml/index.mdx (1 hunks)
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch osterman/base-path-semantics

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.

@github-actions github-actions bot added the size/xl Extra large size PR label Dec 14, 2025
@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
Copy link
Copy Markdown

mergify bot commented Dec 14, 2025

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏

@mergify mergify bot added the conflict This PR has conflicts label Dec 14, 2025
@mergify
Copy link
Copy Markdown

mergify bot commented Dec 14, 2025

Important

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

@mergify mergify bot added the needs-cloudposse Needs Cloud Posse assistance label Dec 14, 2025
@osterman
Copy link
Copy Markdown
Member Author

Recreating with correct base branch

@osterman osterman closed this Dec 14, 2025
@mergify mergify bot removed conflict This PR has conflicts needs-cloudposse Needs Cloud Posse assistance labels Dec 14, 2025
@mergify
Copy link
Copy Markdown

mergify bot commented Dec 14, 2025

⚠️ The sha of the head commit of this PR conflicts with #1872. Mergify cannot evaluate rules on this PR. ⚠️

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

Labels

size/xl Extra large size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stack validation failing in pipelines in Atmos 1.201.0

2 participants