Skip to content

fix(config): preserve configured runtime options - #11550

Merged
jdx merged 2 commits into
jdx:mainfrom
risu729:agent/fix-runtime-option-layering
Aug 5, 2026
Merged

jdx merged 2 commits into
jdx:mainfrom
risu729:agent/fix-runtime-option-layering

Conversation

@risu729

@risu729 risu729 commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

What

Apply configured backend options to explicit runtime tool arguments in both argument ingestion paths, while preserving the normal option precedence:

registry / manifest / alias defaults < config < explicit inline options

Why

Runtime requests parsed from registry-backed tools can already contain default options. The existing merge treated a non-empty option map as proof that the user supplied inline options, so it skipped configured options entirely. The other runtime argument path did not perform this merge at all.

This is a pre-existing bug found while preparing #11481. It is split out so that feature can build on consistent option layering.

Example bug fixed

Given this project config:

[tools]
solidity = { version = "0.8.0", postinstall = "touch /tmp/solidity-postinstall-ran" }

Running mise install solidity@0.8.2 creates an explicit runtime request for Solidity 0.8.2. Solidity already carries the registry default bin = "solc", so before this change the non-empty registry option map caused mise to skip the configured options entirely. The installation completed without running the configured postinstall, leaving /tmp/solidity-postinstall-ran absent.

This was reproduced with the apt-installed mise 2026.8.1. mise x is not a reproducer because it uses ToolsetBuilder, which already preserves configured options; the affected explicit install path uses ToolRequestSetBuilder.

After this change, the configured postinstall is layered on top of the registry default and runs for the explicitly requested version; explicit inline options still take precedence over both.

Test

  • Added a regression test using Solidity's real registry default.
  • Covers registry defaults, configured overrides, config-only options, and explicit inline precedence.
  • mise run lint-fix
  • mise run test:e2e e2e/cli/test_install_postinstall_cli_version

AI-assisted — Tool: Codex; model: openai/gpt-5; version: unavailable.

Summary by CodeRabbit

  • Bug Fixes
    • Runtime tools now consistently apply configured options, registry defaults, and explicit inline settings.
    • Option precedence is handled more reliably when multiple configuration sources are provided.
    • Postinstall hooks now run correctly when installing tools with both CLI and backend options.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Runtime tool arguments now receive configured options through a shared helper. Registry defaults, configured options, and explicit inline options follow consistent precedence rules. Tests cover option layering and postinstall execution.

Changes

Runtime option layering

Layer / File(s) Summary
Apply and validate runtime option overlays
src/toolset/tool_request_set.rs
A shared helper overlays configured options onto runtime and environment-provided arguments. Tests verify registry defaults, configured options, and inline-option precedence.
Cover postinstall installation with inline options
e2e/cli/test_install_postinstall_cli_version
An end-to-end test installs dummy[foo=bar]@latest`` and verifies that the postinstall hook runs.

Estimated code review effort: 3 (Moderate) | ~15–30 minutes

Poem

A rabbit layers options neat,
Config and registry both meet.
Inline choices stay on top,
Postinstall hops without a stop.
Tests confirm the order’s right.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 summarizes the main change: preserving configured runtime options during runtime argument processing.

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.

@risu729
risu729 marked this pull request as ready for review August 4, 2026 20:27
@risu729
risu729 marked this pull request as draft August 4, 2026 20:27
@greptile-apps

greptile-apps Bot commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR preserves configured backend options when explicit runtime versions are processed, while retaining registry, manifest, alias, config, and inline-option precedence.

  • Applies configured options consistently across both runtime argument ingestion paths.
  • Adds unit coverage for registry, config, and inline option layering.
  • Extends the postinstall regression test to cover explicit backend options.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/toolset/tool_request_set.rs Centralizes configured-option layering for explicit runtime requests and verifies that inline options retain highest precedence.
e2e/cli/test_install_postinstall_cli_version Extends the explicit-version postinstall regression test to include a runtime request with backend options.

Reviews (2): Last reviewed commit: "test(install): cover configured runtime ..." | Re-trigger Greptile

@risu729
risu729 marked this pull request as ready for review August 4, 2026 20:31
@jdx
jdx merged commit b1c6824 into jdx:main Aug 5, 2026
29 checks passed
@risu729
risu729 deleted the agent/fix-runtime-option-layering branch August 6, 2026 00:00
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