Skip to content

refactor(config): separate core and backend tool options#9753

Merged
jdx merged 3 commits into
jdx:mainfrom
risu729:refactor/separate-tool-options
May 11, 2026
Merged

refactor(config): separate core and backend tool options#9753
jdx merged 3 commits into
jdx:mainfrom
risu729:refactor/separate-tool-options

Conversation

@risu729

@risu729 risu729 commented May 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Introduce CoreToolOptions, RawBackendOptions, and ToolOptions while keeping ToolVersionOptions as a compatibility alias.
  • Store mise-owned control-plane options under CoreToolOptions and backend-specific TOML values under RawBackendOptions.
  • Move call sites that need the raw backend map to explicit backend option accessors.
  • Update test fixtures/snapshots and add a serde invariant test that core keys do not leak into raw backend options.

Behavior changes

  • None intended or found during review. This is a refactor of internal option storage/access; config parsing, option merging, version rewriting, backend option serialization, and install behavior should remain unchanged.
  • Snapshot updates reflect internal Debug type names only: ToolOptions, CoreToolOptions, and RawBackendOptions.

Review fixes

  • The lenient parser skips only invalid core options instead of dropping all backend options.
  • Template normalization does not rewrite core option values.
  • The supported scalar error message for core option parsing lists datetime values.
  • A parser-contract doc comment explains why some install-time scalar keys are validated by the core parser but stored in raw opts.

Tests

  • cargo test --all-features tool_version_options
  • cargo check --all-features

risu729

This comment was marked as resolved.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request refactors tool options by splitting them into CoreToolOptions (for mise-specific fields like os, depends, and install_env) and RawBackendOptions. It introduces a new ToolOptions struct, centralizes option parsing logic, and updates backends and configuration handling to use this structured approach. Feedback includes a suggestion to update an error message in src/toolset/tool_version_options.rs to correctly list datetime as a supported scalar type for core options.

Comment thread src/toolset/tool_version_options.rs
@greptile-apps

greptile-apps Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR splits the monolithic ToolVersionOptions struct into two clearly-scoped types — CoreToolOptions (mise-owned control-plane fields: os, depends, install_env) and RawBackendOptions (a newtype over IndexMap<String, toml::Value> for backend-specific values) — unified under ToolOptions, with ToolVersionOptions kept as a type alias for compatibility.

  • All existing call sites are updated to use explicit accessors (backend_options(), into_backend_options(), .core.*) or the Deref-into-CoreToolOptions shorthand; snapshot files are updated to reflect the new Debug type names.
  • A new serde invariant test (test_serde_splits_core_and_backend_options) verifies that deserialization correctly routes named core keys into CoreToolOptions and leaves the rest in RawBackendOptions.values, confirming the double-#[serde(flatten)] design works as intended with the toml crate.
  • The lenient option parser now skips only invalid individual core keys rather than discarding all options on first error, and template normalization is guarded by should_normalize_option_template so core field values are not rewritten.

Confidence Score: 5/5

Pure internal refactor with no observable behavior changes; all call sites updated, snapshots regenerated, and new serde invariant test added. Safe to merge.

Every changed file is a mechanical update of field access patterns or test fixtures. The one structural decision — two #[serde(flatten)] fields in ToolOptions — is validated end-to-end by the new test_serde_splits_core_and_backend_options test using the real toml deserializer. No logic changes were made to option routing, merging, or backend installation paths.

No files require special attention.

Important Files Changed

Filename Overview
src/toolset/tool_version_options.rs Core of the refactor: introduces CoreToolOptions, RawBackendOptions, and ToolOptions; adds Deref impls, serde invariant test, and doc comments. Logic appears correct and well-tested.
src/config/config_file/mise_toml.rs Updated insert_core_options to unpack options.core first; should_normalize_option_template guards template rewriting so core fields are not transformed.
src/backend/vfox.rs Call sites updated to use into_backend_options().into_map() and backend_options().as_map() instead of .opts direct field access; changes are mechanical and correct.
src/backend/asdf.rs Single accessor update from .options().install_env to .options().core.install_env; straightforward and correct.
src/registry.rs Wraps bare IndexMap with RawBackendOptions::from(opts) when constructing ToolVersionOptions; correct mechanical update.
src/cli/tool_stub.rs Construction of ToolVersionOptions migrated to explicit CoreToolOptions { os, depends, install_env } struct; correct.
src/backend/spm.rs Test fixtures updated to use .into() to convert bare IndexMap literals to RawBackendOptions; correct.
src/backend/static_helpers.rs Eight identical test-fixture updates: optsopts.into() to satisfy the new RawBackendOptions type; all correct.
src/toolset/mod.rs Re-exports CoreToolOptions, RawBackendOptions, ToolOptions from the new type definitions; correct.
src/config/mod.rs Return types of get_tool_opts and get_tool_opts_with_overrides changed from ToolVersionOptions to ToolOptions; since the two are now a type alias this is a rename only.

Reviews (4): Last reviewed commit: "docs(config): clarify tool option parser..." | Re-trigger Greptile

Comment thread src/toolset/tool_version_options.rs
@risu729

risu729 commented May 9, 2026

Copy link
Copy Markdown
Contributor Author

Greptile nit note: I checked the latest review note about a dead EPHEMERAL_OPT_KEYS entry. I am leaving the entries in place because they are still a defensive/legacy filter for raw option maps that can come from older manifests/cache data or manual construction, even though current parsing routes depends and install_env into CoreToolOptions. The actionable Gemini and Greptile line threads have been applied, replied to, and resolved.

This comment was generated by an AI coding assistant.

risu729

This comment was marked as outdated.

@risu729 risu729 force-pushed the refactor/separate-tool-options branch from 5989502 to 3ce7dfb Compare May 10, 2026 16:03
@risu729 risu729 force-pushed the refactor/separate-tool-options branch from 3ce7dfb to f46c376 Compare May 11, 2026 11:54
@risu729 risu729 marked this pull request as ready for review May 11, 2026 12:15
@jdx jdx merged commit 2bbe897 into jdx:main May 11, 2026
33 checks passed
@risu729 risu729 deleted the refactor/separate-tool-options branch May 11, 2026 15:28
mise-en-dev added a commit that referenced this pull request May 13, 2026
### 🐛 Bug Fixes

- **(backend)** use runtime paths for backend bin dirs by @risu729 in
[#9606](#9606)
- **(ci)** preserve vendor/aqua-registry/ in PPA publish workflow by
@jdx in [#9782](#9782)
- **(ci)** set UTF-8 locale in e2e Docker image by @jdx in
[#9820](#9820)
- **(ci)** pass UTF-8 locale through to e2e tests by @jdx in
[#9823](#9823)
- **(conda)** dedup repodata by archive identifier instead of URL by
@jdx in [#9831](#9831)
- **(github)** use default shell for credential command by @risu729 in
[#9664](#9664)
- **(settings)** distinguish unset known settings from unknown ones by
@jdx in [#9818](#9818)
- **(upgrade)** remove completed progress jobs to prevent duplicate
output by @jdx in [#9779](#9779)
- **(vfox)** resolve GitHub token lazily inside Lua plugins by @jdx in
[#9816](#9816)

### 🚜 Refactor

- **(config)** separate core and backend tool options by @risu729 in
[#9753](#9753)
- **(schema)** reuse env directive property schemas by @risu729 in
[#9651](#9651)

### 📚 Documentation

- **(aliases)** fix Aliased Versions example and drop stale asdf callout
by @jdx in [#9830](#9830)

### ⚡ Performance

- **(aqua)** use phf for baked registry lookups by @risu729 in
[#9763](#9763)
- **(task)** cache per-file content hashes for
source_freshness_hash_contents by @jdx in
[#9819](#9819)

### 🧪 Testing

- **(e2e)** pin aube to known-good version in npm package_manager test
by @jdx in [#9794](#9794)

### 📦 Registry

- replace unsupported exe options by @risu729 in
[#9587](#9587)
- update pi by @garysassano in
[#9792](#9792)

### Chore

- **(ci)** use non-large runners for release builds by @jdx in
[#9786](#9786)
- **(ci)** compare registry PRs from fork point by @risu729 in
[#9643](#9643)
- **(ci)** make build-copr.sh the single source of truth for COPR
chroots by @jdx in [#9788](#9788)
- **(ci)** use crates.io trusted publishing in release-plz by @jdx in
[#9793](#9793)
- **(ci)** remove autofix.ci workflow by @jdx in
[#9801](#9801)
- **(ci)** restore -large runner for Linux release builds by @jdx in
[#9815](#9815)
- **(ci)** add zizmor workflow for github actions security analysis by
@jdx in [#9804](#9804)
- **(ci)** assert mise run render produces no diff by @jdx in
[#9803](#9803)
- **(copr)** publish EL9 builds via centos-stream+epel-next-9 chroot by
@jdx in [#9787](#9787)

### Ci

- remove pull_request_target workflow by @jdx in
[#9799](#9799)
- remove caching from publishing workflows by @jdx in
[#9800](#9800)

### Security

- reject shell metacharacters in version strings and CI inputs by @jdx
in [#9814](#9814)

## 📦 Aqua Registry Updates

### New Packages (11)

- [`Code-Hex/Neo-cowsay`](https://github.com/Code-Hex/Neo-cowsay)
-
[`SonarSource/sonarqube-cli`](https://github.com/SonarSource/sonarqube-cli)
- [`earendil-works/pi`](https://github.com/earendil-works/pi)
- [`hylo-lang/hylo-new`](https://github.com/hylo-lang/hylo-new)
- [`jfernandez/bpftop`](https://github.com/jfernandez/bpftop)
- [`modem-dev/hunk`](https://github.com/modem-dev/hunk)
- [`npm/cli`](https://github.com/npm/cli)
- [`racket/racket/minimal`](https://github.com/racket/racket)
- [`slackapi/slack-cli`](https://github.com/slackapi/slack-cli)
- [`vectordotdev/vector`](https://github.com/vectordotdev/vector)
- [`wasilibs/go-yamllint`](https://github.com/wasilibs/go-yamllint)

### Updated Packages (10)

- [`DataDog/pup`](https://github.com/DataDog/pup)
- [`aquasecurity/trivy`](https://github.com/aquasecurity/trivy)
- [`astral-sh/uv`](https://github.com/astral-sh/uv)
- [`caarlos0/svu`](https://github.com/caarlos0/svu)
-
[`cargo-bins/cargo-binstall`](https://github.com/cargo-bins/cargo-binstall)
- [`foundry-rs/foundry`](https://github.com/foundry-rs/foundry)
- [`gastownhall/beads`](https://github.com/gastownhall/beads)
-
[`gruntwork-io/terragrunt`](https://github.com/gruntwork-io/terragrunt)
- [`pnpm/pnpm`](https://github.com/pnpm/pnpm)
- [`santosr2/TerraTidy`](https://github.com/santosr2/TerraTidy)
3PeatVR pushed a commit to 3PeatVR/mise that referenced this pull request May 14, 2026
### 🐛 Bug Fixes

- **(backend)** use runtime paths for backend bin dirs by @risu729 in
[jdx#9606](jdx#9606)
- **(ci)** preserve vendor/aqua-registry/ in PPA publish workflow by
@jdx in [jdx#9782](jdx#9782)
- **(ci)** set UTF-8 locale in e2e Docker image by @jdx in
[jdx#9820](jdx#9820)
- **(ci)** pass UTF-8 locale through to e2e tests by @jdx in
[jdx#9823](jdx#9823)
- **(conda)** dedup repodata by archive identifier instead of URL by
@jdx in [jdx#9831](jdx#9831)
- **(github)** use default shell for credential command by @risu729 in
[jdx#9664](jdx#9664)
- **(settings)** distinguish unset known settings from unknown ones by
@jdx in [jdx#9818](jdx#9818)
- **(upgrade)** remove completed progress jobs to prevent duplicate
output by @jdx in [jdx#9779](jdx#9779)
- **(vfox)** resolve GitHub token lazily inside Lua plugins by @jdx in
[jdx#9816](jdx#9816)

### 🚜 Refactor

- **(config)** separate core and backend tool options by @risu729 in
[jdx#9753](jdx#9753)
- **(schema)** reuse env directive property schemas by @risu729 in
[jdx#9651](jdx#9651)

### 📚 Documentation

- **(aliases)** fix Aliased Versions example and drop stale asdf callout
by @jdx in [jdx#9830](jdx#9830)

### ⚡ Performance

- **(aqua)** use phf for baked registry lookups by @risu729 in
[jdx#9763](jdx#9763)
- **(task)** cache per-file content hashes for
source_freshness_hash_contents by @jdx in
[jdx#9819](jdx#9819)

### 🧪 Testing

- **(e2e)** pin aube to known-good version in npm package_manager test
by @jdx in [jdx#9794](jdx#9794)

### 📦 Registry

- replace unsupported exe options by @risu729 in
[jdx#9587](jdx#9587)
- update pi by @garysassano in
[jdx#9792](jdx#9792)

### Chore

- **(ci)** use non-large runners for release builds by @jdx in
[jdx#9786](jdx#9786)
- **(ci)** compare registry PRs from fork point by @risu729 in
[jdx#9643](jdx#9643)
- **(ci)** make build-copr.sh the single source of truth for COPR
chroots by @jdx in [jdx#9788](jdx#9788)
- **(ci)** use crates.io trusted publishing in release-plz by @jdx in
[jdx#9793](jdx#9793)
- **(ci)** remove autofix.ci workflow by @jdx in
[jdx#9801](jdx#9801)
- **(ci)** restore -large runner for Linux release builds by @jdx in
[jdx#9815](jdx#9815)
- **(ci)** add zizmor workflow for github actions security analysis by
@jdx in [jdx#9804](jdx#9804)
- **(ci)** assert mise run render produces no diff by @jdx in
[jdx#9803](jdx#9803)
- **(copr)** publish EL9 builds via centos-stream+epel-next-9 chroot by
@jdx in [jdx#9787](jdx#9787)

### Ci

- remove pull_request_target workflow by @jdx in
[jdx#9799](jdx#9799)
- remove caching from publishing workflows by @jdx in
[jdx#9800](jdx#9800)

### Security

- reject shell metacharacters in version strings and CI inputs by @jdx
in [jdx#9814](jdx#9814)

## 📦 Aqua Registry Updates

### New Packages (11)

- [`Code-Hex/Neo-cowsay`](https://github.com/Code-Hex/Neo-cowsay)
-
[`SonarSource/sonarqube-cli`](https://github.com/SonarSource/sonarqube-cli)
- [`earendil-works/pi`](https://github.com/earendil-works/pi)
- [`hylo-lang/hylo-new`](https://github.com/hylo-lang/hylo-new)
- [`jfernandez/bpftop`](https://github.com/jfernandez/bpftop)
- [`modem-dev/hunk`](https://github.com/modem-dev/hunk)
- [`npm/cli`](https://github.com/npm/cli)
- [`racket/racket/minimal`](https://github.com/racket/racket)
- [`slackapi/slack-cli`](https://github.com/slackapi/slack-cli)
- [`vectordotdev/vector`](https://github.com/vectordotdev/vector)
- [`wasilibs/go-yamllint`](https://github.com/wasilibs/go-yamllint)

### Updated Packages (10)

- [`DataDog/pup`](https://github.com/DataDog/pup)
- [`aquasecurity/trivy`](https://github.com/aquasecurity/trivy)
- [`astral-sh/uv`](https://github.com/astral-sh/uv)
- [`caarlos0/svu`](https://github.com/caarlos0/svu)
-
[`cargo-bins/cargo-binstall`](https://github.com/cargo-bins/cargo-binstall)
- [`foundry-rs/foundry`](https://github.com/foundry-rs/foundry)
- [`gastownhall/beads`](https://github.com/gastownhall/beads)
-
[`gruntwork-io/terragrunt`](https://github.com/gruntwork-io/terragrunt)
- [`pnpm/pnpm`](https://github.com/pnpm/pnpm)
- [`santosr2/TerraTidy`](https://github.com/santosr2/TerraTidy)
jdx pushed a commit that referenced this pull request May 15, 2026
## Summary
- Add a shared `BackendOptions` reader for raw and platform-scoped
backend tool options.
- Add a private typed option wrapper for the unified
GitHub/GitLab/Forgejo backend.
- Move GitHub/GitLab/Forgejo option reads and lockfile option collection
behind that wrapper while keeping common install/verify helpers on raw
`ToolVersionOptions`.
- Keep HTTP, S3, Aqua, and UBI migrations out of this PR; those are
split into separate draft follow-ups.
- Defer `Backend::option_specs()` metadata changes to a later PR.

## Project item
-
https://github.com/users/risu729/projects/3/views/1?pane=issue&itemId=185472417
- PR C from project item "Refactor and fix tool option persistence by
phase".
- PR A: #9742, merged.
- PR B: #9753, merged.
- Draft follow-ups currently split from the original PR C scope:
  - HTTP: #9870
  - S3: #9871
  - Aqua: #9872
  - UBI: #9873

## Behavior notes
- This PR is intended to be a refactor and does not change
lockfile/manifest persistence classifications.
- The refactor includes two small behavior fixes found while
centralizing Git backend option reads:
- Forgejo now shares the same git install-time option filtering as
GitHub/GitLab.
- `no_app` is now read through target-aware platform option lookup, so
`platforms.<target>.no_app` works for cross-platform lock resolution.

## Review
- Gemini and Greptile AI review suggestions were addressed and their
review threads are resolved.
- Manual review focused on preserving existing string/scalar coercion
and platform fallback behavior in the Git backend wrapper.

## Verification
- `cargo fmt --all -- --check`
- `git diff --check upstream/main...HEAD`
- GitHub CI is pending for `fda4e0227` after narrowing the PR scope.

*This PR was updated by an AI coding assistant.*
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