fix(node): include source build options in lock identity

[risu729]

Summary

• include Node mirror/flavor and source-build inputs in lock identity
• resolve Node install inputs from tool options first, then settings, then legacy environment fallbacks
• use the same effective Node options when building download URLs, source-build commands, and lockfile option keys

Classification

Mixed actual lock/install identity and stale-lock invalidation fix.

Actual identity: compile, CFLAGS, configure options, make commands/options, and explicit Ninja selection can change the source-built Node output for the same version. A source tarball URL alone does not represent those build inputs.

Stale-lock invalidation: mirror_url and flavor primarily select a different binary archive URL/checksum. Once the lock entry is selected, the locked URL is enough to download it, but these selectors still need to be part of the current exact-options key so a lock entry produced for one mirror/flavor is not reused after config changes.

Settings, tool options, and env fallback

Node install inputs now use this precedence: tool options in [tools], then node.* settings, then the legacy Node build environment variables that mise already honored (NODE_BUILD_MIRROR_URL, NODE_CFLAGS, NODE_CONFIGURE_OPTS, NODE_MAKE_OPTS, and NODE_MAKE_INSTALL_OPTS).

The legacy env fallback is still required for lock identity because the installer still honors those variables during source builds. If they were ignored by lockfile option resolution, changing one of those env vars could reuse a lock entry produced for different source-build inputs. The MISE_NODE_* env vars are covered through normal settings resolution.

Verification

• cargo fmt --check
• git diff --check
• cargo test node_lockfile_options
• cargo test mirror_url_for
• cargo clippy -- -D warnings

[gemini-code-assist]

Code Review

This pull request introduces InstallManifest as a new source for tool options, allowing settings from the install manifest to be incorporated into the option resolution process. It also refactors the Node.js plugin to include additional build configurations, such as mirror_url and cflags, in the lockfile identity. Feedback identifies concerns regarding lockfile portability and stability in the Node.js plugin, specifically pointing out that platform-specific checks and direct environment variable access for build options can cause inconsistent lock identities across different systems.

[greptile-apps]

Greptile Summary

This PR fixes Node.js lock identity by introducing a NodeOptions<'a> struct that centralizes option resolution with a clear precedence chain (tool options → node.* settings → legacy env var fallbacks). Mirror URL, flavor, and all source-build inputs (compile, cflags, configure_opts, make, make_opts, make_install_opts, ninja) are now included in the lockfile identity when relevant, and the same resolved options are used consistently across URL construction, build commands, and lock key generation.

• NodeOptions replaces scattered Settings::get().node.* calls: build-command helpers (configure_cmd, make_cmd, make_install_cmd, ninja, concurrency) are moved from SettingsNode into NodeOptions, which adds the tool-option layer on top and removes the methods from the settings struct.
• Lock identity is now complete for source builds: mirror_url (when non-default) and flavor are always captured; compile inputs are captured under the compile = true guard. Legacy env vars (NODE_CFLAGS, NODE_CONFIGURE_OPTS, NODE_MAKE_OPTS, NODE_MAKE_INSTALL_OPTS) remain in the fallback chain so changing them invalidates stale lock entries.

Confidence Score: 5/5

Safe to merge — the change is a well-scoped refactor with no altered runtime behavior for common paths and comprehensive new tests covering settings-only, tool-options-override, and env-fallback scenarios.

All option resolution paths flow through the new NodeOptions helper, which is exercised end-to-end by the three new node_lockfile_options tests. The previous gap where env-var compile inputs were excluded from lock identity is now closed. The only notable behavioral delta — removing the is_current_platform guard on compile in resolve_lockfile_options — is intentional per the PR description and consistent with the 'same effective options everywhere' goal.

No files require special attention.

Important Files Changed

Filename	Overview
src/plugins/core/node.rs	Major refactor: introduces NodeOptions<'a> to centralize option resolution (tool options → settings → env fallbacks), moves build command helpers out of SettingsNode, and includes mirror_url/flavor/compile-build inputs in lock identity. No bugs found; logic is correct.
src/config/settings.rs	Removes configure_cmd, make_cmd, make_install_cmd, ninja(), and concurrency() helpers from SettingsNode (they moved to NodeOptions in node.rs); cflags() accessor is retained. Corresponding unit tests removed. Clean deletion with no issues.

_{Reviews (8): Last reviewed commit: "fix(node): remove unused settings helper..." | Re-trigger Greptile}

[risu729]

CI note: lint/test-ci are failing because cargo deny check now detects RUSTSEC-2026-0145 for astral-tokio-tar 0.6.1 via rattler_package_streaming. This appears unrelated to this PR: the current main branch is failing the same cargo-deny advisory in the test workflow, and test-ci only failed because lint failed.

This comment was generated by an AI coding assistant.

[coderabbitai]

📝 Walkthrough

Walkthrough

resolve_lockfile_options now always builds lockfile options from settings.node and environment variables; it conditionally includes mirror_url, flavor, and compile=true, and when compile is enabled records source-build inputs (cflags, configure_opts, make, make_opts, make_install_opts, ninja). Tests add controlled Settings reset and assert these options.

Changes

Node Lockfile Options Refactoring

Layer / File(s)	Summary
Lockfile options construction src/plugins/core/node.rs	resolve_lockfile_options now derives options regardless of target.is_current(), conditionally inserts mirror_url when non-default, compile=true when enabled, adds flavor when set, and records source-build inputs (cflags, configure_opts, make, make_opts, make_install_opts, ninja) from settings.node and NODE_* env vars.
Test settings reset helper src/plugins/core/node.rs	Adds a static mutex, a drop guard to reset global Settings, and a helper that applies SettingsPartial, builds a ToolRequest for a current target, and calls NodePlugin::resolve_lockfile_options for deterministic tests.
Unit tests for lockfile options src/plugins/core/node.rs	Adds tests asserting mirror_url and flavor presence when configured, and that enabling compile yields compile=true plus the full set of source-build option keys with configured values.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 I hopped through Settings and code,
Options collected, neatly stowed,
Mirror, flavor, compile in sight,
Build flags gathered, tests say "right",
A tiny hop — the lockfile glows. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% 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
Title check	✅ Passed	The title 'fix(node): include source build options in lock identity' clearly describes the main change: ensuring source build options (mirror, flavor, compile settings) are included in the Node.js lockfile identity.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[risu729]

CI note: the current failures appear unrelated to this PR. e2e-2 fails in tasks/test_task_completion because usage complete-word now returns completion descriptions (mise.toml\tmise.toml, mise.usage.kdl\tmise.usage.kdl) where the test expects only values. e2e-3 fails the same way in tasks/test_task_completion_global_cd (alpha\talpha, beta\tbeta, gamma\tgamma).

I checked another recent non-Node PR/test run (fix(python): include sysconfig patch option in lock identity, run 26718815146) and it fails the same two e2e tranches with the same completion-output mismatch. This PR only changes Node lockfile identity handling in src/plugins/core/node.rs.

This comment was generated by an AI coding assistant.