Skip to content

feat(config): auto type inference + provenance/merge bug fixes - #2897

Open
Erik Osterman (Cloud Posse) (osterman) wants to merge 3 commits into
mainfrom
osterman/field-test-config-commands
Open

feat(config): auto type inference + provenance/merge bug fixes#2897
Erik Osterman (Cloud Posse) (osterman) wants to merge 3 commits into
mainfrom
osterman/field-test-config-commands

Conversation

@osterman

@osterman Erik Osterman (Cloud Posse) (osterman) commented Aug 6, 2026

Copy link
Copy Markdown
Member

what

  • atmos config set and atmos stack set now default --type to auto: infer from the Atmos config schema, then from the type of the value already at the path, falling back to a string (with a warning) only when neither source has an answer. atmos stack set previously never inferred at all — every value was stored as a string unless --type was passed explicitly.
  • Fixed PickProvenanceFile (shared by atmos stack set/get/delete/list and the AI/MCP tools layer) always picking the last provenance entry, which for any value defined only in an imported catalog file was a phantom Line:0 entry pointing at the wrong (importing) manifest instead of the file that actually defines the value.
  • Fixed MCPSettings.Enabled (a bool tagged omitempty) silently disappearing from the merged config when atmos.yaml and an atmos.d/ fragment both set it to different values, instead of the explicit value winning.
  • Fixed error hints containing a raw <placeholder> (e.g. pass --config <file>.) being silently stripped by the terminal markdown renderer, which parses unescaped angle brackets as inline HTML.
  • --config a.yaml,b.yaml on atmos config get/set/delete/format now warns that only the first file is targeted, instead of silently dropping the rest.
  • atmos config get on a key defined only in an atmos.d/ fragment now hints to check atmos describe config instead of just reporting "not found".
  • The unset alias (for config/stack delete) now shows up in --help output, alongside del.
  • Adds a blog post and roadmap entry for the type-inference change, and fixes a stale doc example that cited a non-existent config field.

why

  • A hands-on DX field-test pass of atmos config and atmos stack/atmos stack config surfaced these as real, reproducible bugs and gaps — silent type corruption, a provenance-resolution bug that broke edits for the standard catalog-import stack pattern, and a config value that could vanish entirely on merge.
  • These commands had effectively zero CLI-level test coverage before this PR; the fixes are backed by new regression tests reproducing each bug (including a second, independently-broken copy of the provenance bug found in the AI/MCP tools layer during the fix).

references

  • N/A

Summary by CodeRabbit

  • New Features
    • config set and stack set now automatically infer value types, with warnings when values fall back to strings.
    • Imported catalog values can be set, deleted, and formatted directly in their source file.
    • Help output now displays all available command aliases.
  • Bug Fixes
    • Improved not-found guidance for configuration lookups.
    • Preserved error-hint placeholders such as <file> in formatted output.
    • MCP settings now default safely to disabled and are handled consistently.
  • Documentation
    • Updated command references and examples for automatic type inference and imported configuration values.

…e and merge bugs

Adds --type=auto (the new default) to `atmos config set` and `atmos stack
set`: it infers from the Atmos config schema, then from the type of the
value already at the path, warning instead of silently stringifying when
neither source has an answer. `atmos stack set` previously never inferred
at all.

Also fixes several bugs found during a field-test pass of the config/stack
commands:
- MCPSettings.Enabled (bool + omitempty) could vanish entirely when merging
  atmos.yaml with an atmos.d fragment that both set it; now a *bool.
- PickProvenanceFile picked a phantom Line:0 provenance entry over the real
  defining file for values that live only in an imported catalog manifest,
  breaking `stack set/delete/get/list` for the standard catalog+import
  pattern. This also fixed an independent copy of the same bug in the AI/MCP
  tools layer.
- Error hints containing a raw <placeholder> were silently stripped by the
  markdown renderer (parsed as inline HTML); now HTML-entity-escaped outside
  backtick spans.
- `--config a.yaml,b.yaml` on edit commands silently targeted only the first
  file; now warns.
- `config get` on a key defined only in an atmos.d fragment returned a bare
  "not found" with no indication it might be defined elsewhere.
- `unset` alias was missing from `--help` output for config/stack delete.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@atmos-pro

atmos-pro Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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.

@osterman Erik Osterman (Cloud Posse) (osterman) added the minor New features that do not break anything label Aug 6, 2026
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@osterman, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 20 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 744163fd-cd10-40f6-bb50-a86efbf3e02c

📥 Commits

Reviewing files that changed from the base of the PR and between 7951e3f and e25025c.

📒 Files selected for processing (13)
  • cmd/config/operations.go
  • cmd/config/operations_test.go
  • cmd/stack/operations.go
  • cmd/stack/operations_test.go
  • errors/builder.go
  • errors/builder_test.go
  • pkg/yaml/edit.go
  • pkg/yaml/edit_test.go
  • pkg/yaml/path.go
  • pkg/yaml/typed.go
  • pkg/yaml/typed_test.go
  • website/blog/2026-08-06-config-stack-type-inference.mdx
  • website/docs/cli/commands/config/config-set.mdx
📝 Walkthrough

Walkthrough

The change adds automatic type inference for configuration and stack setters, catalog-aware provenance editing, nullable MCP enablement, safe error hint rendering, and complete subcommand alias display. Documentation, fixtures, and regression tests cover the new behavior.

Changes

Configuration and stack value management

Layer / File(s) Summary
YAML type inference primitives
pkg/yaml/edit.go, pkg/yaml/typed.go, pkg/yaml/*_test.go
Adds automatic type constants, YAML value detection, file-based type lookup, and string-fallback classification.
Config setter inference
cmd/config/operations.go, cmd/config/operations_test.go, website/docs/cli/commands/config/config-set.mdx
config set infers types from the schema and existing values, warns on string fallback, and warns when multiple --config files are supplied.
Stack setter and provenance editing
cmd/stack/*, pkg/stack/*, pkg/ai/tools/atmos/stack_config_*_test.go, tests/fixtures/scenarios/config-field-test/*, website/docs/cli/commands/stack/*, website/blog/*, website/src/data/roadmap.js
stack set uses automatic inference. Import-only values resolve to catalog manifests for set, delete, list, and format operations.

MCP enablement state

Layer / File(s) Summary
Nullable MCP configuration
pkg/schema/mcp.go, pkg/config/mcp_enabled_merge_test.go, pkg/datafetcher/schema/atmos/config/1.0.json
MCP enablement is nullable and defaults to false when unset. Explicit root values survive configuration merging.
MCP command integration
cmd/mcp/client/*, cmd/mcp/server/start.go
MCP commands use IsEnabled() and pointer-based assignment for interactive enablement.

Error and help output

Layer / File(s) Summary
Error hint sanitization
errors/builder.go, errors/builder_test.go, errors/formatter_test.go
Ordinary hint angle brackets are HTML-escaped. Backtick spans, EXAMPLE:, and TITLE: hints remain unchanged.
Subcommand alias rendering
cmd/help_template.go, cmd/help_template_test.go
Help output renders every alias for each subcommand.

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

Possibly related PRs

Suggested reviewers: aknysh

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 39.66% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 the primary type-inference changes and the related provenance and merge bug fixes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch osterman/field-test-config-commands

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (1)
pkg/yaml/edit_test.go (1)

156-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use table-driven tests for the new type-inference scenarios.

Each site tests independent scenarios with repeated setup and assertions. Use named table cases so each type and fallback case runs independently.

  • pkg/yaml/edit_test.go#L156-L173: Table-drive boolean, integer, string, float, null, and missing-path cases.
  • pkg/yaml/edit_test.go#L251-L265: Table-drive successful lookup, missing-path, and missing-file cases.
  • cmd/stack/operations_test.go#L400-L463: Table-drive existing-value inference and new-key string-fallback cases.

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 `@pkg/yaml/edit_test.go` around lines 156 - 173, Convert the independent
scenarios into named table-driven tests: in pkg/yaml/edit_test.go lines 156-173,
cover boolean, integer, string, float, null, and missing-path cases; in
pkg/yaml/edit_test.go lines 251-265, cover successful lookup, missing-path, and
missing-file cases; and in cmd/stack/operations_test.go lines 400-463, cover
existing-value inference and new-key string fallback. Run each case through the
relevant existing test logic with per-case assertions.

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 `@errors/builder.go`:
- Around line 191-218: Update escapeHintAngleBrackets to use a delimiter-aware
scanner that preserves Markdown code spans with one or more backticks, including
``--file <manifest>``; stop relying on backtickSpanPattern’s single-backtick
regex and ensure angle brackets inside every valid code span remain unchanged.
Add the requested regression case to TestEscapeHintAngleBrackets.

In `@pkg/yaml/edit.go`:
- Around line 199-230: Update GetType to detect path presence without calling
Get, so explicit YAML null values remain distinguishable from missing or
unaddressable nodes. Extend its tag mapping to return TypeNull for !!null while
preserving existing type mappings and failure behavior. Add a regression test
covering auto inference for an existing explicit null value.

In `@pkg/yaml/typed.go`:
- Line 42: Extend looksNonStringPattern to recognize common numeric CLI forms
including scientific notation (such as 1e3), an explicit plus sign (such as +5),
and leading-decimal values (such as .5), while retaining existing booleans,
integers, and decimals. Add regression cases covering these forms and verify
type inference no longer treats them as strings without the fallback warning.

In `@website/blog/2026-08-06-config-stack-type-inference.mdx`:
- Around line 42-46: Update both atmos stack set examples to include the
required -s <stack> and -c <component> selectors, preserving their existing
variable values and type option.
- Around line 45-46: Correct both fallback-warning examples: in
website/blog/2026-08-06-config-stack-type-inference.mdx lines 45-46, either
remove --type=bool to demonstrate automatic fallback or revise the comment to
describe explicit boolean typing; in
website/docs/cli/commands/config/config-set.mdx lines 44-46, remove the warning
claim for production or change the example to use a bool- or number-like value
on a new unmodeled path.

---

Nitpick comments:
In `@pkg/yaml/edit_test.go`:
- Around line 156-173: Convert the independent scenarios into named table-driven
tests: in pkg/yaml/edit_test.go lines 156-173, cover boolean, integer, string,
float, null, and missing-path cases; in pkg/yaml/edit_test.go lines 251-265,
cover successful lookup, missing-path, and missing-file cases; and in
cmd/stack/operations_test.go lines 400-463, cover existing-value inference and
new-key string fallback. Run each case through the relevant existing test logic
with per-case assertions.
🪄 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: 857569fc-1397-41bb-9a73-e5342425d3dd

📥 Commits

Reviewing files that changed from the base of the PR and between 3ce4349 and 8dc615c.

📒 Files selected for processing (36)
  • cmd/config/operations.go
  • cmd/config/operations_test.go
  • cmd/help_template.go
  • cmd/help_template_test.go
  • cmd/mcp/client/add.go
  • cmd/mcp/client/add_test.go
  • cmd/mcp/client/install.go
  • cmd/mcp/client/list.go
  • cmd/mcp/client/status.go
  • cmd/mcp/client/uninstall.go
  • cmd/mcp/server/start.go
  • cmd/stack/config.go
  • cmd/stack/operations.go
  • cmd/stack/operations_test.go
  • errors/builder.go
  • errors/builder_test.go
  • errors/formatter_test.go
  • pkg/ai/tools/atmos/stack_config_common_test.go
  • pkg/ai/tools/atmos/stack_config_delete_test.go
  • pkg/ai/tools/atmos/stack_config_format_test.go
  • pkg/ai/tools/atmos/stack_config_list_test.go
  • pkg/ai/tools/atmos/stack_config_set_test.go
  • pkg/config/mcp_enabled_merge_test.go
  • pkg/datafetcher/schema/atmos/config/1.0.json
  • pkg/schema/mcp.go
  • pkg/stack/edit.go
  • pkg/stack/edit_test.go
  • pkg/yaml/edit.go
  • pkg/yaml/edit_test.go
  • pkg/yaml/typed.go
  • pkg/yaml/typed_test.go
  • website/blog/2026-08-06-config-stack-type-inference.mdx
  • website/docs/cli/commands/config/config-set.mdx
  • website/docs/cli/commands/stack/config/set.mdx
  • website/docs/cli/commands/stack/stack-set.mdx
  • website/src/data/roadmap.js

Comment thread errors/builder.go Outdated
Comment thread pkg/yaml/edit.go
Comment thread pkg/yaml/typed.go Outdated
Comment thread website/blog/2026-08-06-config-stack-type-inference.mdx Outdated
Comment thread website/blog/2026-08-06-config-stack-type-inference.mdx Outdated
Adds the durable Atmos project fixture used to manually verify the
config/stack fixes in this PR live: multi-level imports, base-component
inheritance, YAML anchors/aliases, and an atmos.d fragment -- coverage no
existing fixture provided for this command surface.

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

Copy link
Copy Markdown
Member Author

Reopening to re-trigger CI checks, which never registered on initial open.

@atmos-pro

atmos-pro Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Note

Atmos Pro  

Waiting for your GitHub Actions workflow to upload affected stacks.
Learn More.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

- GetType now correctly distinguishes an explicit YAML null (real,
  present value) from a missing path, returning (TypeNull, true)
  instead of ok=false. The auto-inference callers (config/stack set)
  deliberately still treat an existing null as unresolved, since
  buildRHS's TypeNull case ignores the value argument and would
  otherwise silently discard the new value being set.
- looksNonStringPattern now recognizes scientific notation (1e3),
  explicit plus signs (+5), and leading/trailing-dot decimals (.5, 5.),
  so the fallback-to-string warning fires for these shapes too.
- escapeHintAngleBrackets now uses a CommonMark-compliant
  delimiter-aware code-span scanner instead of a single-backtick regex,
  so double-backtick-delimited hints are handled correctly.
- Fixed two doc examples (blog post + config-set.mdx) that claimed to
  demonstrate the fallback warning but used commands that couldn't
  actually produce it (explicit --type, or a value that doesn't look
  non-string); also added the required -s/-c flags to two stack set
  examples.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor New features that do not break anything size/l Large size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant