Skip to content

fix(config): reload settings with config - #10945

Merged
jdx merged 6 commits into
jdx:mainfrom
risu729:agent/bootstrap-preserve-settings
Jul 12, 2026
Merged

jdx merged 6 commits into
jdx:mainfrom
risu729:agent/bootstrap-preserve-settings

Conversation

@risu729

@risu729 risu729 commented Jul 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • invalidate file-derived settings whenever Config::reset() reloads configuration
  • rediscover settings paths so newly created config files participate in the reload
  • preserve CLI settings while rebuilding the settings and backend caches
  • cover bootstrap configs installed by the dotfiles phase before automatic deps run for the final task

Root cause

Config::reset() reloaded config files but retained the separately cached Settings. Settings also read from a process-static config-path list, so clearing only the values would not discover a global config created during bootstrap. Later phases could therefore observe newly loaded tasks and deps through Config while feature gates still read an older settings snapshot, causing deps is experimental even though the installed configuration enabled experimental features.

The reload now invalidates settings before backend/tool state is rebuilt, ensuring backends and the subsequent config load use the same settings snapshot.

User impact

Bootstrap phases now use one consistent configuration snapshot. Settings introduced or changed by earlier bootstrap phases are visible to later hooks, tools, deps, and tasks without discarding command-line overrides.

Validation

  • mise run lint-fix
  • cargo test reload
  • cargo check --all-features
  • mise run test:e2e e2e/cli/test_bootstrap

Found while testing risu729/dotfiles#3702.

Summary by CodeRabbit

  • Bug Fixes
    • Configuration reset now reloads settings after internal cleanup and before reloading configuration.
    • Reloading settings refreshes file-based configuration while preserving command-line overrides.
    • Configuration file discovery now uses the supported TOML configuration paths consistently.
  • Tests
    • Added coverage to ensure reset reloads settings correctly and that reload preserves command-line overrides.

@coderabbitai

coderabbitai Bot commented Jul 11, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Settings::reload() refreshes file-backed configuration while preserving CLI overrides. Config::reset() invokes it before loading a new configuration, with tests covering both behaviors.

Changes

Settings reset integration

Layer / File(s) Summary
Settings reload contract
src/config/settings.rs
Settings file discovery uses TOML_CONFIG_FILENAMES; Settings::reload() clears file-loaded state and the config-root cache while preserving CLI settings, with unit coverage.
Reset and reload integration
src/config/mod.rs
Config::reset() reloads settings before loading the new configuration, with a test verifying that a new settings instance is created.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit hops through settings bright,
Reloading files by morning light.
CLI flags stay snug in place,
Reset begins with careful grace.
Tests nod softly: “All is right!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.33% 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
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 clearly matches the main change: reloading settings during config reset.

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.

@greptile-apps

greptile-apps Bot commented Jul 11, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes Config::reset() so that settings derived from config files are invalidated and rediscovered on every reload, ensuring bootstrap phases that install new config files (e.g. ~/.config/mise/config.toml with experimental = true) are visible to subsequent operations without losing CLI-level overrides.

  • all_settings_files() now calls load_config_paths() instead of the static-lazy ALL_TOML_CONFIG_FILES, so clearing GLOBAL_CONFIG_FILES in the reset causes fresh path discovery on the next Settings::get().
  • Settings::reload() is inserted in Config::reset() after the timeout closure clears the path caches, ensuring Config::load() rebuilds settings from the new snapshot while CLI_SETTINGS is untouched.
  • Unit tests cover both behaviors: pointer inequality after Config::reset() and preservation of CLI-supplied settings through Settings::reload().

Confidence Score: 5/5

The change is narrowly scoped to config-reload ordering and does not touch any hot paths outside of the bootstrap reset flow.

The three-step ordering in Config::reset() is correct and matches the semantics described. CLI_SETTINGS is deliberately untouched by reload(), confirmed by the new unit test. The switch from ALL_TOML_CONFIG_FILES to load_config_paths() enables re-discovery. No pre-existing callers are broken.

No files require special attention.

Important Files Changed

Filename Overview
src/config/settings.rs Adds Settings::reload() that clears BASE_SETTINGS without touching CLI_SETTINGS; switches all_settings_files() from the static ALL_TOML_CONFIG_FILES lazy to load_config_paths(), enabling re-discovery after GLOBAL_CONFIG_FILES is cleared. New unit test verifies CLI overrides survive the reload.
src/config/mod.rs Inserts Settings::reload() in Config::reset() after the timeout closure clears GLOBAL_CONFIG_FILES/SYSTEM_CONFIG_FILES, ensuring Config::load() rebuilds settings from freshly discovered paths. New async test asserts the Arc pointer changes after reset.

Reviews (6): Last reviewed commit: "fix(config): reload settings after clear..." | Re-trigger Greptile

Comment thread e2e/cli/test_bootstrap Resolved
@risu729
risu729 marked this pull request as ready for review July 11, 2026 13:52
@github-actions

Copy link
Copy Markdown
Contributor

This PR currently has failing checks. If this continues for 7 days, it will be closed automatically.

This is warning day 1 of 7.

Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it.

This comment was generated by an automated workflow.

Comment thread src/config/mod.rs Resolved
@risu729
risu729 marked this pull request as draft July 12, 2026 04:18
@risu729
risu729 marked this pull request as ready for review July 12, 2026 04:33
@jdx
jdx merged commit 0cb66b8 into jdx:main Jul 12, 2026
33 checks passed
@risu729
risu729 deleted the agent/bootstrap-preserve-settings branch July 12, 2026 14:06
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.

2 participants