fix(config): honor --config across internal reloads and multi-file merges

[Erik Osterman (Cloud Posse) (osterman)]

what

• Internal reloads of the CLI config (many call sites across internal/exec, pkg/vendoring, cmd/, etc. calling InitCliConfig(schema.ConfigAndStacksInfo{}, false)) now fall back to parsing --config/--config-path/--base-path from os.Args/env instead of silently discarding the selection made at startup.
• A second --config file that sets a conflicting value for an array-typed key (e.g. stacks.included_paths) no longer aborts stack discovery for entries that still legitimately match; a real "nothing matched at all" case now returns a distinct error instead.
• atmos config get now reports the effective, fully-merged configuration for the invocation (all --config files, --config-path dirs, and profiles applied) instead of reading a single physical file.
• VendorDirAbsolutePath/WorkflowsDirAbsolutePath are now precomputed once (mirroring the existing top-level base_path resolution), so vendor/workflow path joins no longer re-derive a possibly still-relative BasePath.

why

• atmos --config <file> terraform plan/test was failing with failed to find import even though atmos --config <file> list stacks worked with the identical flag, because a downstream InitCliConfig re-invocation lost the --config selection mid-command.
• Splitting config across two --config files with a conflicting array value made stacks.included_paths unusable for stack discovery, while atmos config get misleadingly reported the config as unchanged.

references

Closes #2867
Closes #2868

[atmos-pro]

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

[github-actions]

Resource Changes Found for bucket in test

Plan: 4 to add, 0 to change, 0 to destroy.

To reproduce this locally, run:

atmos terraform plan bucket -s test

---

Create

+ aws_s3_bucket.checkov_target
+ aws_s3_bucket.this
+ aws_s3_bucket.trivy_target
+ aws_s3_bucket_public_access_block.trivy_target

Terraform Plan Summary

  # aws_s3_bucket.checkov_target will be created
  + resource "aws_s3_bucket" "checkov_target" {
      + acceleration_status         = (known after apply)
      + acl                         = (known after apply)
      + arn                         = (known after apply)
      + bucket                      = "atmos-native-ci-e2e-checkov-test"
      + bucket_domain_name          = (known after apply)
      + bucket_prefix               = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = false
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + object_lock_enabled         = (known after apply)
      + policy                      = (known after apply)
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + tags_all                    = (known after apply)
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)

      + cors_rule (known after apply)

      + grant (known after apply)

      + lifecycle_rule (known after apply)

      + logging (known after apply)

      + object_lock_configuration (known after apply)

      + replication_configuration (known after apply)

      + server_side_encryption_configuration (known after apply)

      + versioning (known after apply)

      + website (known after apply)
    }

  # aws_s3_bucket.this will be created
  + resource "aws_s3_bucket" "this" {
      + acceleration_status         = (known after apply)
      + acl                         = (known after apply)
      + arn                         = (known after apply)
      + bucket                      = "atmos-native-ci-e2e-test"
      + bucket_domain_name          = (known after apply)
      + bucket_prefix               = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = false
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + object_lock_enabled         = (known after apply)
      + policy                      = (known after apply)
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + tags                        = {
          + "AtmosFixture" = "native-ci-e2e"
          + "Stage"        = "test"
        }
      + tags_all                    = {
          + "AtmosFixture" = "native-ci-e2e"
          + "Stage"        = "test"
        }
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)

      + cors_rule (known after apply)

      + grant (known after apply)

      + lifecycle_rule (known after apply)

      + logging (known after apply)

      + object_lock_configuration (known after apply)

      + replication_configuration (known after apply)

      + server_side_encryption_configuration (known after apply)

      + versioning (known after apply)

      + website (known after apply)
    }

  # aws_s3_bucket.trivy_target will be created
  + resource "aws_s3_bucket" "trivy_target" {
      + acceleration_status         = (known after apply)
      + acl                         = (known after apply)
      + arn                         = (known after apply)
      + bucket                      = "atmos-native-ci-e2e-trivy-test"
      + bucket_domain_name          = (known after apply)
      + bucket_prefix               = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = false
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + object_lock_enabled         = (known after apply)
      + policy                      = (known after apply)
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + tags_all                    = (known after apply)
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)

      + cors_rule (known after apply)

      + grant (known after apply)

      + lifecycle_rule (known after apply)

      + logging (known after apply)

      + object_lock_configuration (known after apply)

      + replication_configuration (known after apply)

      + server_side_encryption_configuration (known after apply)

      + versioning (known after apply)

      + website (known after apply)
    }

  # aws_s3_bucket_public_access_block.trivy_target will be created
  + resource "aws_s3_bucket_public_access_block" "trivy_target" {
      + block_public_acls       = true
      + block_public_policy     = true
      + bucket                  = (known after apply)
      + id                      = (known after apply)
      + ignore_public_acls      = true
      + restrict_public_buckets = true
    }

Plan: 4 to add, 0 to change, 0 to destroy.

Changes to Outputs:
  + bucket_name = "atmos-native-ci-e2e-test"

[coderabbitai]

📝 Walkthrough

Walkthrough

This change makes configuration loading use the effective merged configuration, preserves source-aware path resolution, improves workflow and vendor path handling, updates stack discovery errors, and adds hook, command, documentation, and regression-test coverage.

Changes

Configuration resolution

Layer / File(s)	Summary
Multi-source configuration and discovery pkg/config/..., pkg/schema/schema.go, pkg/flags/..., pkg/datafetcher/schema/..., tests/snapshots/...	Explicit config files, config paths, environment values, profiles, and array merges now use a shared loading path. Relative paths retain their declaring source directory. Stack discovery distinguishes empty matches from genuine glob errors.
Effective configuration commands cmd/config/..., pkg/config/config_edit.go, pkg/mcp/config/..., website/docs/cli/commands/config/...	config get reads merged configuration values. Single-file editing commands reject multiple config files with an invalid-argument error.
Absolute vendor and workflow paths internal/exec/..., pkg/vendoring/resolve.go, pkg/config/config.go, pkg/schema/schema.go	Vendor and workflow paths use precomputed absolute values when available. Error messages use normalized display paths while retaining paths needed for resolution.
Hook paths and supporting updates pkg/hooks/..., docs/prd/..., .claude/skills/field-test/SKILL.md, cmd/config/schema_test.go	Step hook working directories now classify bare, dot-prefixed, and absolute paths. Related requirements, testing guidance, and helper names were updated.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant LoadConfig
  participant Profiles
  participant AtmosConfiguration
  participant StackDiscovery
  CLI->>LoadConfig: select config files, paths, and environment values
  LoadConfig->>Profiles: merge profile sources and overrides
  Profiles-->>LoadConfig: return effective configuration
  LoadConfig->>AtmosConfiguration: resolve source-aware base, vendor, and workflow paths
  AtmosConfiguration->>StackDiscovery: provide merged included paths
  StackDiscovery-->>CLI: return manifests or structured discovery errors

Loading

Possibly related PRs

• cloudposse/atmos#2664: Both changes update config get behavior and its command tests.
• cloudposse/atmos#2772: Both changes track configuration-file provenance for relative path resolution.
• cloudposse/atmos#2864: Both changes update CLI configuration loading and base-path resolution.

Suggested reviewers: aknysh

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The PR also changes hook working-directory semantics, field-test skill behavior, profile documentation, and unrelated vendor/workflow path handling.	Move unrelated hook, field-test, profile, and vendor/workflow changes into separate pull requests or link issues that require them.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes preserving --config during internal reloads and supporting multi-file configuration merges.
Linked Issues check	✅ Passed	The changes address CLI selection preservation, array merging, distinct no-match errors, and effective config get output [#2867, #2868].
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch osterman/issues-2867-2868

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (5)

cmd/config/operations_test.go (1)

154-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider sharing one test streams stub in this package.

The comment states this mirrors configSchemaTestStreams in schema_test.go. Both types are in package config and implement the same five methods identically. Go allows one shared stub for both files.

Move a single configTestStreams into a shared test helper file in this package and delete both copies. Not a blocker.

🤖 Prompt for 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.

In `@cmd/config/operations_test.go` around lines 154 - 166, Replace the duplicate
configGetTestStreams and configSchemaTestStreams implementations with one shared
configTestStreams stub in a package-level test helper file. Update both test
files to use configTestStreams and remove the redundant type and methods while
preserving the existing stdio reader and buffer behavior.

pkg/config/multifile_array_merge_test.go (1)

124-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Turn the stage 2 and stage 3 checkpoints into assertions, or drop them.

Stages 2 and 3 capture values and only log them. They verify nothing. The doc comment explains they existed to locate where the value diverged, and that investigation is now complete — the fix landed in pkg/config/utils.go, not in the merge.

Both stages have a known expected value now. Assert it, so a future regression in mergeConfigFile or mergeImports fails the test instead of writing a line to the log.

♻️ Assert the intermediate values

 	// Stage 2: after merging fragment.yaml on top (mergeConfigFile only, no mergeImports yet).
 	require.NoError(t, mergeConfigFile(fragmentFile, v))
-	afterMergeConfigFile := v.Get("stacks.included_paths")
-	t.Logf("stage 2 (after mergeConfigFile(fragment.yaml)): %#v", afterMergeConfigFile)
+	require.Equal(t, []interface{}{"deploy/**/*", "other/**/*"}, v.Get("stacks.included_paths"),
+		"stage 2: MergeConfig must fully replace the array, not union or revert it")
 
 	// Stage 3: after mergeImports runs (a no-op for files with no `import:` key, per
 	// processConfigImportsWithFSAndBasePathSource's early return -- confirming that,
 	// or finding it ISN'T a no-op, is the point of this checkpoint).
 	_, err := mergeImports(v, tmpDir, "", "")
 	require.NoError(t, err)
-	afterMergeImports := v.Get("stacks.included_paths")
-	t.Logf("stage 3 (after mergeImports): %#v", afterMergeImports)
+	require.Equal(t, []interface{}{"deploy/**/*", "other/**/*"}, v.Get("stacks.included_paths"),
+		"stage 3: mergeImports is a no-op here and must not disturb the merged array")
 
 	// Stage 4: after the final v.Unmarshal into the typed struct (what loadConfigFromCLIArgs
 	// itself does last).
 	var atmosConfig schema.AtmosConfiguration
 	require.NoError(t, v.Unmarshal(&atmosConfig, atmosDecodeHook()))
-	t.Logf("stage 4 (after v.Unmarshal): %#v", atmosConfig.Stacks.IncludedPaths)

🤖 Prompt for 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.

In `@pkg/config/multifile_array_merge_test.go` around lines 124 - 135, Update the
stage 2 and stage 3 checkpoints in the test to assert their known expected
values instead of only logging afterMergeConfigFile and afterMergeImports. Keep
the existing mergeConfigFile and mergeImports calls, and retain the final
behavior checks while making regressions in either intermediate stage fail the
test.

pkg/config/load_config_args.go (1)

61-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider retiring loadConfigFromCLIArgs once its tests move to the real path.

LoadConfig no longer calls this function. Only tests do. The doc comment says so directly, which is honest, but it leaves a second, partial copy of the config tail in production code.

This copy already diverges from LoadConfig's tail. It skips promoteAtmosEnvFromConfig, profile loading, restoreCaseSensitiveCommandEnvMaps, the profiles.base_path sync, bridgeVendorUpdaterConfig, and the container-runtime bridge. A test that passes here no longer proves the production path behaves the same way, and the gap will widen as the real tail grows.

pkg/config/multifile_array_merge_test.go already exercises the production path through InitCliConfig in three of its tests. If the two remaining loadConfigFromCLIArgs callers move to LoadConfig or InitCliConfig, this function can be deleted. Not a blocker for this PR.

🤖 Prompt for 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.

In `@pkg/config/load_config_args.go` around lines 61 - 69, Retire the unused
production-only loadConfigFromCLIArgs path after migrating its remaining tests
to exercise LoadConfig or InitCliConfig, preserving equivalent test coverage
through the real configuration flow. Remove the function and any now-unused
references or imports, while leaving mergeConfigFromCLIArgs and the existing
production tail unchanged.

pkg/config/utils.go (1)

163-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting the shared glob-resolution block.

This block is now byte-identical to lines 57-78 in FindAllStackConfigsInPathsForStack, including the hint text, the context keys, and the sentinel check. The comment manages that by pointing at the sibling, which works only as long as somebody remembers to update both.

A small helper would remove the duplication:

// resolveStackGlobMatches returns the stack manifests matching p, or nil when p is
// valid but currently matches nothing.
func resolveStackGlobMatches(atmosConfig *schema.AtmosConfiguration, p string) ([]string, error) {
	patterns := []string{p}
	if filepath.Ext(p) == "" {
		patterns = getStackFilePatterns(p, true)
	}

	var allMatches []string
	for _, pattern := range patterns {
		matches, err := u.GetGlobMatches(pattern)
		if err == nil && len(matches) > 0 {
			allMatches = append(allMatches, matches...)
		}
	}
	if len(allMatches) > 0 {
		return allMatches, nil
	}

	// GetGlobMatches reports "pattern matched nothing" by wrapping ErrFailedToFindImport.
	// That is expected and must not abort discovery for the other entries
	// (cloudposse/atmos#2867). Only a different underlying error is genuine.
	if _, err := u.GetGlobMatches(patterns[0]); err != nil && !errors.Is(err, errUtils.ErrFailedToFindImport) {
		return nil, errUtils.Build(err).
			WithHintf("Verify `stacks.base_path` in `atmos.yaml` points to the correct directory").
			WithHint("Check that the stacks directory exists and contains stack configuration files").
			WithContext("pattern", patterns[0]).
			WithContext("stacks_base_path", atmosConfig.StacksBaseAbsolutePath).
			Err()
	}
	return nil, nil
}

Both callers then reduce to a call plus a continue when the result is empty. Note the ForStack variant takes atmosConfig by value, so it would pass &atmosConfig.

As per coding guidelines: "Before implementing functionality, search internal/exec/ and pkg/ and extend existing code rather than duplicating it."

🤖 Prompt for 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.

In `@pkg/config/utils.go` around lines 163 - 179, The glob-resolution and
error-handling logic is duplicated between the current caller and
FindAllStackConfigsInPathsForStack. Extract it into a shared
resolveStackGlobMatches helper using the existing pattern expansion,
GetGlobMatches, sentinel check, and contextual error construction, then update
both callers to use it and continue when no matches are returned; pass the
ForStack configuration by address as needed.

Source: Coding guidelines

pkg/config/config_test.go (1)

542-561: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the effective path-resolution contract in the regression test.

The new case sets BasePath to baseDir, which is already absolute. It therefore cannot detect the failure mode where a consumer incorrectly re-joins a raw relative base path. Add table-driven cases that use the repository’s CliConfigPath convention, assert filepath.IsAbs, and verify both derived paths under the resolved base. Add explicit expectations for empty and absolute nested values, or reject unsupported absolute values in the implementation.

As per coding guidelines and the PR objectives, new Go features require comprehensive, behavior-focused unit tests for the effective path-resolution case. Based on learnings, do not assume a path join treats an absolute second component as an override; test the contract or enforce relative values.

🤖 Prompt for 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.

In `@pkg/config/config_test.go` around lines 542 - 561, Expand the table-driven
regression coverage around AtmosConfigAbsolutePaths to use relative and absolute
CliConfigPath-style base paths, asserting filepath.IsAbs and the expected
VendorDirAbsolutePath and WorkflowsDirAbsolutePath under the resolved base.
Include explicit cases for empty and absolute nested BasePath values, or update
the implementation to reject unsupported absolute nested values and test that
behavior.

Sources: Coding guidelines, Learnings

🤖 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 `@cmd/config/operations_test.go`:
- Around line 193-203: Add viper.Reset protection to the affected test in the
setup around configGetCmd.RunE: reset the global Viper before the test runs and
register t.Cleanup(viper.Reset) afterward, matching the pattern used by the
multifile array merge tests. Add the required viper import.

In `@internal/exec/validate_component.go`:
- Around line 38-43: Update getWorkflowsDirToUse in
internal/exec/validate_component.go and the corresponding Vendor.BasePath
fallback in internal/exec/vendor_utils.go to reuse the existing absolute-aware
base-path resolution helper instead of directly joining raw BasePath values.
Preserve the configured absolute override behavior while ensuring relative and
absolute child paths resolve consistently under the configured top-level base
path.

In `@pkg/config/config.go`:
- Around line 532-542: Update the filepath.Abs error handling in the vendor and
workflow path-resolution code to wrap failures with repository-standard static
errors and contextual %w messages identifying whether vendor or workflow
resolution failed. Preserve the existing early returns and assignments while
applying the wrapper consistently to both error branches.

In `@pkg/config/load.go`:
- Around line 407-412: The profile-loading flow must not treat the
semicolon-delimited result of connectPaths as one directory. Update
discoverProfileLocations or its caller to split CliConfigPath into individual
CLI config directories and search each contributor separately, preserving
single-path behavior; alternatively validate and reject multiple CLI config
directories before profile loading.

In `@pkg/config/multifile_array_merge_test.go`:
- Line 263: Update the test cleanup around ATMOS_PROFILE in the affected test to
preserve its original environment value: capture whether it was set and its
value before the test changes it, then restore that exact state with deferred
cleanup, including unsetting it only when it was originally absent. Replace the
direct os.Unsetenv cleanup while keeping the existing error handling.
- Around line 252-257: Update the opening identifier in the doc comment above
TestInitCliConfig_ProfileAppliedOnTopOfConfigFlag to match that exact test
function name, leaving the remainder of the comment unchanged.

---

Nitpick comments:
In `@cmd/config/operations_test.go`:
- Around line 154-166: Replace the duplicate configGetTestStreams and
configSchemaTestStreams implementations with one shared configTestStreams stub
in a package-level test helper file. Update both test files to use
configTestStreams and remove the redundant type and methods while preserving the
existing stdio reader and buffer behavior.

In `@pkg/config/config_test.go`:
- Around line 542-561: Expand the table-driven regression coverage around
AtmosConfigAbsolutePaths to use relative and absolute CliConfigPath-style base
paths, asserting filepath.IsAbs and the expected VendorDirAbsolutePath and
WorkflowsDirAbsolutePath under the resolved base. Include explicit cases for
empty and absolute nested BasePath values, or update the implementation to
reject unsupported absolute nested values and test that behavior.

In `@pkg/config/load_config_args.go`:
- Around line 61-69: Retire the unused production-only loadConfigFromCLIArgs
path after migrating its remaining tests to exercise LoadConfig or
InitCliConfig, preserving equivalent test coverage through the real
configuration flow. Remove the function and any now-unused references or
imports, while leaving mergeConfigFromCLIArgs and the existing production tail
unchanged.

In `@pkg/config/multifile_array_merge_test.go`:
- Around line 124-135: Update the stage 2 and stage 3 checkpoints in the test to
assert their known expected values instead of only logging afterMergeConfigFile
and afterMergeImports. Keep the existing mergeConfigFile and mergeImports calls,
and retain the final behavior checks while making regressions in either
intermediate stage fail the test.

In `@pkg/config/utils.go`:
- Around line 163-179: The glob-resolution and error-handling logic is
duplicated between the current caller and FindAllStackConfigsInPathsForStack.
Extract it into a shared resolveStackGlobMatches helper using the existing
pattern expansion, GetGlobMatches, sentinel check, and contextual error
construction, then update both callers to use it and continue when no matches
are returned; pass the ForStack configuration by address as needed.

🪄 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: dcd92e57-1fcf-4aa6-a1ea-396b818750c8

📥 Commits

Reviewing files that changed from the base of the PR and between d2b8e81 and 472dc36.

📒 Files selected for processing (28)

• cmd/config/operations.go
• cmd/config/operations_test.go
• internal/exec/describe_workflows_test.go
• internal/exec/validate_component.go
• internal/exec/vendor_utils.go
• internal/exec/workflow.go
• internal/exec/workflow_utils.go
• pkg/config/base_path_resolution_test.go
• pkg/config/config.go
• pkg/config/config_test.go
• pkg/config/load.go
• pkg/config/load_config_args.go
• pkg/config/multifile_array_merge_test.go
• pkg/config/utils.go
• pkg/datafetcher/schema/atmos/config/1.0.json
• pkg/schema/schema.go
• pkg/vendoring/resolve.go
• tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.golden
• tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden
• tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden
• tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden
• tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden
• tests/snapshots/TestCLICommands_atmos_workflow_file_not_found.stderr.golden
• tests/snapshots/TestCLICommands_atmos_workflow_invalid_manifest.stderr.golden
• tests/snapshots/TestCLICommands_indentation.stdout.golden
• tests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden
• website/docs/cli/commands/config/config-get.mdx
• website/docs/cli/configuration/profiles.mdx

[codecov]

Codecov Report

❌ Patch coverage is 92.33449% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.80%. Comparing base (4aec494) to head (47e19e8).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/config/load.go	86.66%	6 Missing and 2 partials ⚠️
pkg/config/load_config_args.go	89.47%	4 Missing and 2 partials ⚠️
pkg/config/config.go	71.42%	2 Missing and 2 partials ⚠️
internal/exec/vendor_utils.go	90.47%	2 Missing ⚠️
internal/exec/workflow_utils.go	75.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2875      +/-   ##
==========================================
+ Coverage   82.79%   82.80%   +0.01%     
==========================================
  Files        1863     1863              
  Lines      180652   180790     +138     
==========================================
+ Hits       149568   149707     +139     
+ Misses      23293    23286       -7     
- Partials     7791     7797       +6     

Flag	Coverage Δ
unittests	82.80% <92.33%> (+0.01%)	⬆️

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

Files with missing lines	Coverage Δ
cmd/config/operations.go	83.33% <100.00%> (+3.33%)	⬆️
internal/exec/validate_component.go	65.21% <100.00%> (+0.88%)	⬆️
internal/exec/validate_schema.go	80.00% <100.00%> (+0.90%)	⬆️
internal/exec/workflow.go	74.38% <100.00%> (ø)
pkg/config/config_edit.go	81.63% <100.00%> (+2.56%)	⬆️
pkg/config/profiles.go	85.56% <100.00%> (+0.91%)	⬆️
pkg/config/utils.go	88.80% <100.00%> (+0.50%)	⬆️
pkg/flags/global_registry.go	99.35% <100.00%> (+0.01%)	⬆️
pkg/hooks/step_engine.go	87.87% <100.00%> (+0.57%)	⬆️
pkg/mcp/config/config.go	88.00% <100.00%> (+0.32%)	⬆️
... and 7 more

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 6

Caution

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

⚠️ Outside diff range comments (2)

pkg/config/load_config_args.go (1)

51-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Track profiles.base_path provenance for --config-path.

profilesBasePathConfigDir only comes from --config files. A later --config-path configuration can override profiles.base_path, but profile discovery still resolves the final value against an earlier --config directory.

Return declaration provenance from mergeConfigFromDirectories. Apply the directory from the last merged configuration that declares profiles.base_path. Add coverage for mixed --config and --config-path input.

🤖 Prompt for 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.

In `@pkg/config/load_config_args.go` around lines 51 - 58, Update the
config-loading flow around mergeConfigFromDirectories and
profilesBasePathConfigDir to return and apply the directory provenance of
profiles.base_path from the last merged --config-path configuration that
declares it, overriding earlier --config provenance. Extend coverage to verify
mixed --config and --config-path inputs resolve profiles from the later
declaration.

pkg/config/load.go (1)

398-407: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fill fallback selection fields independently.

The fallback runs only when all three fields are empty. If an internal caller supplies AtmosBasePath but omits AtmosConfigFilesFromArg, --config and ATMOS_CONFIG are ignored. The same problem applies to other partial selections.

Read the fallback once. Fill each empty field without replacing a caller-provided field. Add regression cases for each partial-selection combination.

🤖 Prompt for 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.

In `@pkg/config/load.go` around lines 398 - 407, The fallback logic around
getConfigSelectionFromFlagsOrEnv must run when any selection field is missing,
not only when all three are empty. Read the fallback once, then independently
populate each empty AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, and
AtmosBasePath field while preserving caller-provided values; add regression
coverage for every partial-selection combination.

🤖 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 @.claude/skills/field-test/SKILL.md:
- Around line 3-4: Update the field-test targeting algorithm in the skill
instructions to resolve the actual pull-request base when available, rather than
relying on the upstream tracking branch. Compare that base against the complete
worktree, including staged, unstaged, and untracked changes, and inspect git
status before prompting for a target when no explicit target is provided.
Preserve the existing explicit-target behavior and investigation-only scope.
- Around line 45-50: Update the implementation-inspection guidance in the
field-test skill to require inspecting every changed production package
identified by the diff, including internal/exec/ when present, rather than
treating it only as phased-out context. Explicitly retain the requirement to
inspect both internal/exec/ and pkg/ when determining implementation scope or
extending functionality, while also checking thin cmd/<command>/ call sites and
relevant error paths.

In `@internal/exec/validate_schema.go`:
- Around line 372-375: Update relPath to reject only paths whose first relative
component is the parent-directory segment "..", while preserving relative names
such as "..vendor.yaml" and other in-directory files. Add a regression test
covering an in-directory filename beginning with ".." and verify displayPath
keeps it relative.

In `@internal/exec/vendor_utils_test.go`:
- Around line 1376-1380: Update the “ErrEmptySources” test case in the vendor
execution tests so its atmosVendorSpec includes a valid, non-empty Imports
configuration that resolves to zero sources, avoiding the missing-definition
guard in ExecuteAtmosVendorInternal. Assert that the returned error matches
ErrEmptySources via ErrorIs, while preserving the existing empty-sources setup
and test intent.

In `@pkg/config/load_profile_test.go`:
- Line 211: Update the comment for TestParseViperProfilesFromEnv_Quirks to end
with a period, without changing its wording or surrounding test code.

In `@website/docs/cli/configuration/configuration.mdx`:
- Around line 32-33: Update the configuration source description to explicitly
document precedence: command-line flags such as --config and --config-path
override their environment variable equivalents, followed by config files and
then defaults.

---

Outside diff comments:
In `@pkg/config/load_config_args.go`:
- Around line 51-58: Update the config-loading flow around
mergeConfigFromDirectories and profilesBasePathConfigDir to return and apply the
directory provenance of profiles.base_path from the last merged --config-path
configuration that declares it, overriding earlier --config provenance. Extend
coverage to verify mixed --config and --config-path inputs resolve profiles from
the later declaration.

In `@pkg/config/load.go`:
- Around line 398-407: The fallback logic around
getConfigSelectionFromFlagsOrEnv must run when any selection field is missing,
not only when all three are empty. Read the fallback once, then independently
populate each empty AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, and
AtmosBasePath field while preserving caller-provided values; add regression
coverage for every partial-selection combination.

🪄 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: 63dd978a-6688-4746-9e8d-ea2299308980

📥 Commits

Reviewing files that changed from the base of the PR and between 571f0d2 and e98a7c5.

📒 Files selected for processing (27)

• .claude/skills/field-test/SKILL.md
• cmd/config/operations.go
• cmd/config/operations_test.go
• internal/exec/describe_workflows_test.go
• internal/exec/validate_schema.go
• internal/exec/validate_schema_test.go
• internal/exec/vendor_utils.go
• internal/exec/vendor_utils_test.go
• internal/exec/workflow_utils.go
• pkg/config/config_edit.go
• pkg/config/import_base_path_test.go
• pkg/config/load.go
• pkg/config/load_config_args.go
• pkg/config/load_error_paths_unix_test.go
• pkg/config/load_profile_test.go
• pkg/config/load_test.go
• pkg/config/multifile_array_merge_test.go
• pkg/config/profiles.go
• pkg/flags/global_registry.go
• pkg/flags/global_registry_test.go
• pkg/mcp/config/config.go
• pkg/mcp/config/config_test.go
• pkg/schema/schema.go
• website/docs/cli/commands/config/config-delete.mdx
• website/docs/cli/commands/config/config-format.mdx
• website/docs/cli/commands/config/config-set.mdx
• website/docs/cli/configuration/configuration.mdx

🚧 Files skipped from review as they are similar to previous changes (5)

• internal/exec/workflow_utils.go
• pkg/config/profiles.go
• pkg/schema/schema.go
• cmd/config/operations.go
• internal/exec/vendor_utils.go

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

internal/exec/vendor_utils_test.go (1)

1211-1249: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use table-driven cases for the path-resolution scenarios.

Both test functions define multiple scenarios with repeated subtest setup and assertions. Use a test-case slice and a loop for each function.

As per coding guidelines, “Use table-driven tests for testing multiple scenarios in Go.”

🤖 Prompt for 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.

In `@internal/exec/vendor_utils_test.go` around lines 1211 - 1249, Convert
TestGetVendorDirToUse and TestResolveVendorConfigFilePath_CheckGlobalConfig to
table-driven tests using case slices and subtest loops. Keep each existing
scenario’s AtmosConfiguration setup, invocation, and expected path unchanged
while moving the repeated assertions into the loop.

Source: Coding guidelines

🤖 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 @.claude/skills/field-test/SKILL.md:
- Around line 23-25: Update the field-test change-inspection instructions to
include the full committed diff, not only its stat summary, and explicitly read
the contents of every untracked path returned by git ls-files --others
--exclude-standard. Preserve inspection of staged and unstaged changes via git
diff HEAD so target inference covers all changed implementation files.

In `@cmd/config/operations_test.go`:
- Around line 243-257: Update TestConfigGetCommand_InitCliConfigError to create
and use cmd.NewTestKit(t) before invoking configGetCmd.RunE, ensuring the Cobra
command test isolates and cleans shared root-command state while preserving the
existing malformed-config assertions.

In `@internal/exec/vendor_utils_test.go`:
- Around line 1212-1218: Update the vendor path tests around getVendorDirToUse
to use OS-native paths: create temporary roots with t.TempDir(), construct paths
via filepath.Join, and derive expected values from those constructed paths.
Ensure the absolute Vendor.BasePath case exercises filepath.IsAbs correctly on
every platform, including Windows.

---

Nitpick comments:
In `@internal/exec/vendor_utils_test.go`:
- Around line 1211-1249: Convert TestGetVendorDirToUse and
TestResolveVendorConfigFilePath_CheckGlobalConfig to table-driven tests using
case slices and subtest loops. Keep each existing scenario’s AtmosConfiguration
setup, invocation, and expected path unchanged while moving the repeated
assertions into the loop.

🪄 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: 2c80a02a-96b0-4ee4-a482-6a3fd1d373bf

📥 Commits

Reviewing files that changed from the base of the PR and between e98a7c5 and 13d7399.

📒 Files selected for processing (10)

• .claude/skills/field-test/SKILL.md
• cmd/config/operations_test.go
• internal/exec/validate_schema.go
• internal/exec/validate_schema_test.go
• internal/exec/vendor_utils_test.go
• pkg/config/base_path_resolution_test.go
• pkg/config/config_edit_test.go
• pkg/config/multifile_array_merge_test.go
• pkg/config/profiles_test.go
• website/docs/cli/configuration/configuration.mdx

🚧 Files skipped from review as they are similar to previous changes (3)

• internal/exec/validate_schema.go
• pkg/config/profiles_test.go
• internal/exec/validate_schema_test.go

[coderabbitai]

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 @.claude/skills/field-test/SKILL.md:
- Around line 23-29: Update the base-resolution flow in the field-test
instructions before invoking git diff <base>...HEAD: validate that the selected
PR base or main fallback resolves to an existing Git ref, preferring
origin/<base>, <base>, or main as appropriate. Distinguish an unfetched PR
branch from lookup failure, and stop requesting user input when no usable base
ref exists.

🪄 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: fc30a384-22d2-4555-992f-6289609ee9c1

📥 Commits

Reviewing files that changed from the base of the PR and between 13d7399 and dda284e.

📒 Files selected for processing (2)

• .claude/skills/field-test/SKILL.md
• internal/exec/vendor_utils_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/exec/vendor_utils_test.go

[coderabbitai]

Actionable comments posted: 2

🤖 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 @.claude/skills/field-test/SKILL.md:
- Around line 23-29: Update the base-resolution instructions in the field-test
workflow to preserve a known pull-request base: resolve the PR target branch by
commit or its remote/local ref, and if unavailable, ask the user for the base
instead of falling back to origin/main or main. Only use the repository default
branch when no pull request exists, while retaining the existing verification
before running git diff.

In `@docs/prd/atmos-profiles.md`:
- Around line 1362-1433: Update the Phase 1 configuration loading-chain
description near the precedence logic to match FR3.1: use FR3.1 as the
authoritative reference or reorder the sources to its defined precedence, and
explicitly state that --config and --config-path sources bypass subsequent
discovery. Remove the conflicting duplicate order while preserving the profiles
placement relative to .atmos.d/ and local atmos.yaml.

🪄 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: cb34919e-417a-4562-ad48-7868679b3d37

📥 Commits

Reviewing files that changed from the base of the PR and between 13d7399 and 3779b9b.

⛔ Files ignored due to path filters (1)

• website/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (15)

• .claude/skills/field-test/SKILL.md
• docs/prd/atmos-profiles.md
• docs/prd/base-path-resolution-semantics.md
• docs/prd/custom-hooks.md
• internal/exec/vendor_utils_test.go
• pkg/config/base_path_resolution_test.go
• pkg/config/config.go
• pkg/config/load.go
• pkg/config/load_config_args.go
• pkg/config/multifile_array_merge_test.go
• pkg/config/utils.go
• pkg/hooks/step_engine.go
• pkg/hooks/step_engine_test.go
• pkg/schema/schema.go
• website/package.json

🚧 Files skipped from review as they are similar to previous changes (5)

• internal/exec/vendor_utils_test.go
• pkg/config/load.go
• pkg/config/base_path_resolution_test.go
• pkg/config/load_config_args.go
• pkg/config/utils.go

[coderabbitai]

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)

.claude/skills/field-test/SKILL.md (1)

17-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not treat gh lookup failures as “no PR.”

gh pr view can fail because of missing authentication, network errors, or an unavailable CLI. If the skill treats that failure as “no PR,” it can compare against the wrong base. The origin/main/main fallback also fails for repositories whose default branch is develop or trunk.

Treat a successful empty result as “no PR.” When gh is unavailable, resolve the default branch from local Git metadata such as origin/HEAD; otherwise ask for the base explicitly.

Also applies to: 31-35

🤖 Prompt for 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.

In @.claude/skills/field-test/SKILL.md around lines 17 - 18, Update the
branch-resolution instructions around gh pr view so only a successful empty
result means no PR; distinguish lookup failures such as unavailable gh,
authentication, or network errors and do not silently fall back to a guessed
branch. When no PR exists, resolve the default branch from local metadata such
as origin/HEAD, and if that is unavailable require the base branch explicitly
instead of assuming origin/main or main.

🧹 Nitpick comments (2)

.claude/skills/field-test/SKILL.md (2)

83-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Include configuration documentation in Phase 1.

The required scope names only website/docs/cli/commands/. Configuration and profile behavior is also documented under website/docs/cli/configuration/. Search the full website/docs/cli/ tree so field tests cover the documented configuration contract.

🤖 Prompt for 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.

In @.claude/skills/field-test/SKILL.md around lines 83 - 86, Update the Phase 1
documentation scope in the field-test skill guidance to search the full
website/docs/cli/ tree, including website/docs/cli/configuration/ alongside
command documentation, so configuration and profile behavior are included in the
documented contract review.

---

66-70: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Keep one target for one cross-layer feature.

A single command flow can span cmd/, pkg/, and internal/exec/. Splitting it by package can test isolated pieces without testing the integration contract. Group related packages under one target. Use separate targets only for unrelated features.

🤖 Prompt for 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.

In @.claude/skills/field-test/SKILL.md around lines 66 - 70, Update the
target-scoping guidance in the default-current-branch flow to group related
changes across packages such as cmd/, pkg/, and internal/exec/ into one
cross-layer feature target. Create separate Phase 2-4 targets only when the
branch contains unrelated features, while retaining separate targets for
genuinely distinct commands or packages.

🤖 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/config/utils.go`:
- Around line 42-72: Update the pattern-processing loop around GetGlobMatches to
handle each returned error immediately: ignore only errors matching
errUtils.ErrFailedToFindImport, and wrap and return every other error using the
current pattern and existing stack-path context. Remove the later retry against
patterns[0], since errors from every generated pattern must be preserved and
reported from the originating iteration.

---

Outside diff comments:
In @.claude/skills/field-test/SKILL.md:
- Around line 17-18: Update the branch-resolution instructions around gh pr view
so only a successful empty result means no PR; distinguish lookup failures such
as unavailable gh, authentication, or network errors and do not silently fall
back to a guessed branch. When no PR exists, resolve the default branch from
local metadata such as origin/HEAD, and if that is unavailable require the base
branch explicitly instead of assuming origin/main or main.

---

Nitpick comments:
In @.claude/skills/field-test/SKILL.md:
- Around line 83-86: Update the Phase 1 documentation scope in the field-test
skill guidance to search the full website/docs/cli/ tree, including
website/docs/cli/configuration/ alongside command documentation, so
configuration and profile behavior are included in the documented contract
review.
- Around line 66-70: Update the target-scoping guidance in the
default-current-branch flow to group related changes across packages such as
cmd/, pkg/, and internal/exec/ into one cross-layer feature target. Create
separate Phase 2-4 targets only when the branch contains unrelated features,
while retaining separate targets for genuinely distinct commands or packages.

🪄 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: cab56d2c-7a53-4731-9cbe-f4bd3ce0a190

📥 Commits

Reviewing files that changed from the base of the PR and between 3779b9b and 131ade0.

📒 Files selected for processing (9)

• .claude/skills/field-test/SKILL.md
• cmd/config/operations_test.go
• cmd/config/schema_test.go
• docs/prd/atmos-profiles.md
• internal/exec/validate_component.go
• internal/exec/vendor_utils.go
• pkg/config/config_test.go
• pkg/config/multifile_array_merge_test.go
• pkg/config/utils.go

🚧 Files skipped from review as they are similar to previous changes (6)

• internal/exec/validate_component.go
• cmd/config/operations_test.go
• pkg/config/config_test.go
• docs/prd/atmos-profiles.md
• internal/exec/vendor_utils.go
• pkg/config/multifile_array_merge_test.go

[coderabbitai]

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/config/base_path_resolution_test.go`:
- Around line 814-832: Extend the test around resolveStackGlobMatches with
multiple patterns: make an earlier pattern valid but unmatched and a later
pattern syntactically invalid, then assert the returned error includes the later
failing pattern in its safe details. Keep the existing ErrFailedToFindImport
assertion and context-detail verification, ensuring the test would fail if error
reporting still uses patterns[0].

🪄 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: da89dd51-c8bb-40a0-b89f-8f7b4af9b390

📥 Commits

Reviewing files that changed from the base of the PR and between 131ade0 and 376700a.

📒 Files selected for processing (3)

• pkg/config/base_path_resolution_test.go
• pkg/config/utils.go
• pkg/schema/schema.go

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/config/utils.go

[atmos-pro]

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

[atmos-pro]

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

[github-actions]

These changes were released in v1.226.0-rc.4.