Skip to content

fix(config): warn for deprecated settings from sources - #10832

Merged
jdx merged 1 commit into
mainfrom
codex/deprecated-settings-warnings
Jul 7, 2026
Merged

jdx merged 1 commit into
mainfrom
codex/deprecated-settings-warnings

Conversation

@jdx

@jdx jdx commented Jul 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • add setting env var names to SettingsMeta so runtime warnings can use settings.toml metadata
  • warn when deprecated settings are explicitly configured via [settings] or process env vars
  • warn for the [env] MISE_TERA_V1 compatibility shim without treating every [env] MISE_* value as a current settings source
  • avoid consuming deprecation warning IDs before the logger is initialized

Follow-up to #10830.

Tests

  • cargo test config::settings::tests
  • cargo build
  • git diff --check
  • smoke checked deprecated [settings] and process-env warnings with shorthands_file

This PR was generated by an AI coding assistant.


Note

Medium Risk
Touches startup ordering for every CLI invocation and hook-env fast exit; incorrect timing could miss warnings or emit them before log-level flags apply, but scope is limited to deprecation messaging rather than config values.

Overview
Improves deprecated setting warnings so they fire when users actually set deprecated options in TOML, process environment, or the MISE_TERA_V1 [env] shim—not only during internal migrations.

SettingsMeta now carries each setting’s env var name (codegen from settings.toml). Warnings are queued until the logger is ready, then flushed from logger::init (after CLI flags are applied) or on hook-env fast exit so dedupe IDs are not consumed too early and messages respect -q / --silent / --log-level.

Config parsing scans [settings] (including nested keys like aqua.registry_url), skips empty/unset values, and treats [env] MISE_TERA_V1 as deprecated without flagging unrelated MISE_* entries in [env]. Process env checks use metadata env keys for deprecated settings.

hook-env fast path no longer skips when those logging flags are present, so warnings can still run after a minimal logger init.

Reviewed by Cursor Bugbot for commit 7e4d31a. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes

    • Improved deprecated-setting detection in config files, including nested/dotted keys, with warnings suppressed when values are unset/empty.
    • Deprecated warnings now better align with the actual related environment variables (including directive-derived cases), reducing false positives.
    • Deprecated warnings are now emitted even on early exit, and early-exit behavior is adjusted to respect logging-suppression flags.
  • New Features

    • Settings can now be linked to specific environment-variable keys to provide more accurate deprecation warnings.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

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

Use the following commands to manage reviews:

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

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds env metadata to settings generation, detects deprecated settings from TOML and environment-derived configuration, queues warnings before logger setup, and flushes them during logger and CLI startup paths.

Changes

Settings env metadata and warning flushing

Layer / File(s) Summary
Generate env field in SettingsMeta build output
build.rs
build.rs emits env: Some(...) or env: None for top-level and nested SettingsMeta entries from TOML env properties.
SettingsMeta field and warning queue
src/config/settings.rs
SettingsMeta gains env; deprecated warnings gain pending and ready-state coordination, and deprecated-key detection/emission now handles TOML shapes, runtime env values, and package-file deprecations.
Parse and flush deprecated warnings
src/config/settings.rs
Settings::parse_settings_file now queues deprecated keys from raw TOML, and the new flush APIs mark warnings ready, emit env-derived warnings, and drain queued keys.
Startup and fast-exit wiring
src/logger.rs, src/cli/mod.rs, src/hook_env.rs
Logger init, CLI fast-exit handling, and preclap logging-flag checks coordinate initialization and warning flushing before returning.
Deprecated-setting detection tests
src/config/settings.rs, src/hook_env.rs
Unit tests cover nested and dotted TOML deprecated keys, MISE_TERA_V1 detection from env directives, and has_preclap_logging_flag matches and non-matches.

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

Poem

A rabbit found some settings old,
With env clues tucked in lines of gold,
It hopped through logs both bright and dim,
Then flushed the warnings off the rim,
🐇 clean config trails for bunny whim!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: warning about deprecated settings from configuration sources.

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.

Comment thread src/config/settings.rs Resolved
@greptile-apps

greptile-apps Bot commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds runtime warnings when deprecated settings are explicitly configured—via [settings] keys, the [env] MISE_TERA_V1 shim, or matching process environment variables—and defers those warnings until the logger is properly initialised so they are never silently lost or emitted before log-level flags have been parsed.

  • SettingsMeta gains an env field (generated in build.rs) that maps each setting to its canonical env-var name, enabling warn_deprecated_env_settings to scan the actual process environment at flush time.
  • Queuing + flushing machinery (PENDING_DEPRECATED_SETTINGS, DEPRECATED_WARNINGS_READY, flush_deprecated_warnings, flush_deprecated_warnings_for_fast_exit) ensures warnings collected during early config parsing are emitted exactly once, after the logger and CLI flags are ready.
  • Fast-exit hook-env path now initialises the logger and flushes pending warnings before returning; a new has_preclap_logging_flag guard prevents the fast-exit when suppression flags (-q, --quiet, --silent, --log-level) are present so those flags can actually take effect.

Confidence Score: 5/5

Safe to merge; changes are limited to deprecation detection and warning timing with no impact on auth, install, or data-path behaviour.

The queuing/flushing flow is carefully ordered: warnings collected during early config parsing are reliably drained after the logger and CLI flags are ready, in both the normal and fast-exit paths. The deduplication set in warn_deprecated_now prevents any double-emission. No correctness or security issues were found.

src/config/settings.rs — the new queueing infrastructure is the most complex addition; the only note is the misleading name of queue_deprecated_settings.

Important Files Changed

Filename Overview
src/config/settings.rs Core change: adds deprecated-setting detection from TOML config and process env, with queuing until logger is ready; minor naming confusion in queue_deprecated_settings
src/cli/mod.rs Fast-exit path now initializes logger and flushes pending deprecated warnings before returning; no issues found
src/hook_env.rs Adds has_preclap_logging_flag to disable fast exit when suppression/log-level flags are present, with tests; correctly excludes --debug/--trace which increase rather than suppress output
build.rs Emits env field in generated SETTINGS_META entries for both top-level and nested settings, matching existing deprecated-fields pattern
src/logger.rs Calls Settings::flush_deprecated_warnings() at end of init() so pending warnings are emitted once the logger is configured; guarded by CLI_SETTINGS check to avoid premature flush on the early logger::init call

Reviews (4): Last reviewed commit: "fix(config): warn for deprecated setting..." | Re-trigger Greptile

Comment thread src/config/settings.rs Resolved

@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: 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 `@src/config/settings.rs`:
- Line 426: Move the deprecation warning calls out of Settings::try_get and into
the startup path after logger::init() so they are actually visible on normal
startup. Specifically, keep warn_deprecated_env_settings and any TOML
deprecation warning logic from running before logging is initialized, and ensure
BASE_SETTINGS does not cache the pre-logger warning side effects by deferring
those warnings until after initialization.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 4903f2bf-119f-4153-ad91-55a7e44c7105

📥 Commits

Reviewing files that changed from the base of the PR and between e7ad2f1 and dadfc7e.

📒 Files selected for processing (2)
  • build.rs
  • src/config/settings.rs

Comment thread src/config/settings.rs Resolved
@jdx
jdx force-pushed the codex/deprecated-settings-warnings branch 2 times, most recently from c368c8b to 4ac5223 Compare July 7, 2026 13:16
@jdx

jdx commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

Addressed the review feedback in 4ac5223b2:

  • queued deprecated setting keys discovered before logger initialization and flushed them from logger::init() after log::set_max_level
  • moved the process-env deprecation scan out of Settings::try_get() and into that post-logger flush path
  • narrowed warn_default_package_file_deprecated so it only short-circuits when SETTINGS_META has deprecation metadata, preserving the fallback warning path

Validated with cargo test config::settings::tests, cargo build, git diff --check, and smoke checks for [settings], process env, and child-only [env] behavior.

This comment was generated by an AI coding assistant.

@jdx
jdx enabled auto-merge (squash) July 7, 2026 13:17
Comment thread src/config/settings.rs Resolved
Comment thread src/logger.rs Resolved
@jdx
jdx disabled auto-merge July 7, 2026 13:20

@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: 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 `@src/logger.rs`:
- Line 195: Move the queued deprecation warning flush in logger::init() so it
happens before the hook_env::should_exit_early_fast() early return path, because
hook_env::should_exit_early_fast() can call Settings::get() and exit before the
logger is initialized. Keep the fix centered around logger::init() and
Settings::flush_deprecated_warnings() so warnings are emitted even when the
fast-exit path is taken.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 0915931c-0198-4932-895f-508614fd122c

📥 Commits

Reviewing files that changed from the base of the PR and between dadfc7e and c368c8b.

📒 Files selected for processing (3)
  • build.rs
  • src/config/settings.rs
  • src/logger.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • build.rs

Comment thread src/logger.rs Resolved
Comment thread src/config/settings.rs Resolved
@jdx
jdx force-pushed the codex/deprecated-settings-warnings branch from 4ac5223 to 4676888 Compare July 7, 2026 13:23
@jdx

jdx commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

Addressed the latest Bugbot feedback in 46768883a:

  • deprecated keys from TOML are now computed before deserialization but only queued after the config successfully deserializes
  • deprecated warnings now stay queued until CLI settings have been applied, so -q/--quiet can suppress them
  • process-env deprecated settings are scanned during the post-CLI logger flush

Validated with cargo test config::settings::tests, cargo build, git diff --check, plus smoke checks for normal [settings], quiet [settings], invalid config, normal process env, and quiet process env behavior.

This comment was generated by an AI coding assistant.

@jdx
jdx enabled auto-merge (squash) July 7, 2026 13:23
Comment thread src/config/settings.rs Resolved
@jdx
jdx force-pushed the codex/deprecated-settings-warnings branch from 4676888 to ac5d1cf Compare July 7, 2026 13:25
@jdx

jdx commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

Addressed the next feedback in ac5d1cf54:

  • validated TOML deprecation keys now go through warn_deprecated, so post-init parse_settings_file callers warn immediately instead of leaving keys queued
  • hook-env fast-exit now initializes the logger and flushes queued deprecation warnings before returning

Revalidated with cargo test config::settings::tests, cargo build, git diff --check, and smoke checks for normal/quiet [settings], invalid config, and quiet process-env behavior.

This comment was generated by an AI coding assistant.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ac5d1cf. Configure here.

Comment thread src/cli/mod.rs Resolved
@jdx
jdx disabled auto-merge July 7, 2026 13:30
@jdx
jdx force-pushed the codex/deprecated-settings-warnings branch from ac5d1cf to 7e4d31a Compare July 7, 2026 13:36
@jdx

jdx commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

Addressed the latest Bugbot feedback in 7e4d31a60:

  • hook-env fast-path now declines early exit when CLI quiet/silent/log-level flags are present, so clap can apply warning suppression before any deprecated-setting flush
  • added unit coverage for the pre-clap logging flag guard while keeping trace/debug/verbose on the existing fast path

Validated with cargo test hook_env::tests, cargo test config::settings::tests, cargo build, git diff --check, and a smoke check showing a stable hook-env still warns normally while hook-env --quiet and hook-env --log-level error suppress deprecated-setting warnings.

This comment was generated by an AI coding assistant.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.7.1 x -- echo 23.0 ± 2.7 20.1 39.7 1.00
mise x -- echo 24.1 ± 1.9 21.0 47.3 1.05 ± 0.15

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.7.1 env 21.2 ± 1.2 18.9 27.6 1.00
mise env 22.1 ± 1.6 19.3 28.8 1.04 ± 0.09

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.7.1 hook-env 22.5 ± 1.7 19.6 28.4 1.00
mise hook-env 25.5 ± 1.5 22.6 32.5 1.13 ± 0.11
⚠️ Inconclusive: hook-env measured 13% slower, but the relative uncertainty overlaps the 10% threshold.

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.7.1 ls 20.7 ± 1.3 17.3 28.3 1.00
mise ls 21.2 ± 1.3 17.9 25.2 1.02 ± 0.09

xtasks/test/perf

Command mise-2026.7.1 mise Variance
install (cached) 173ms 173ms +0%
ls (cached) 63ms 65ms -3%
bin-paths (cached) 70ms 72ms -2%
task-ls (cached) 135ms 135ms +0%

@jdx
jdx merged commit 9b2e496 into main Jul 7, 2026
34 checks passed
@jdx
jdx deleted the codex/deprecated-settings-warnings branch July 7, 2026 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant