Skip to content

Pin shell in installer-generated env setup (Schniz#1525) #199

Open
Dargon789 wants to merge 10 commits into
Dargon789:patch-1from
Schniz:master
Open

Pin shell in installer-generated env setup (Schniz#1525) #199
Dargon789 wants to merge 10 commits into
Dargon789:patch-1from
Schniz:master

Conversation

@Dargon789

@Dargon789 Dargon789 commented Apr 5, 2026

Copy link
Copy Markdown
Owner

Summary by Sourcery

Improve shell integration, architecture support, and installer behavior while preparing the 1.39.0 release.

New Features:

  • Add support for the x64-glibc-217 architecture via a new Arch variant and corresponding --arch value.

Bug Fixes:

  • Ensure the Windows CMD use-on-cd hook works correctly with paths containing spaces and with drive changes by quoting the cd macro path and using cd /d.
  • Prevent duplicate zsh use-on-cd hooks when the environment script is sourced multiple times.
  • Make env --use-on-cd immediately apply the current directory Node version so shells do not need an extra fnm use subprocess on startup.

Enhancements:

  • Have fnm env with --use-on-cd internally invoke use with the multishell path injected, reducing shell startup overhead and improving hook robustness.
  • Prefer explicit --shell flags in installer-generated shell setup to avoid runtime shell inference and process tree detection.
  • Clarify the interactive fnm use missing-version prompt by prefixing the error message with fnm.
  • Derive Clone for FnmConfig and Directories to support passing augmented configuration (such as multishell path) where needed.

CI:

  • Fix installation-script ARM CI by specifying the appropriate Docker platforms for arm64 and armv7 images and tidying workflow formatting.

Documentation:

  • Document the recommendation to pass an explicit shell to fnm env and update examples in README and command docs accordingly.
  • Update the changelog for the 1.39.0 release with the new env --use-on-cd behavior, install --use flag, hook robustness, and toolchain bump.

Tests:

  • Add end-to-end tests for env --use-on-cd to verify immediate version activation and correct behavior when sourcing env twice.
  • Add unit tests for zsh and Windows CMD use-on-cd hooks to assert correct hook de-duplication and path quoting behavior.

Chores:

  • Bump the crate and npm package versions from 1.38.1 to 1.39.0 and add changeset entries describing the new behavior and architecture support.

Schniz and others added 6 commits March 5, 2026 15:47
* fix(zsh): dedupe use-on-cd hook on re-source

* fix(wincmd): support spaces and drive switches in use-on-cd

* test(e2e): cover use-on-cd after re-sourcing env

* chore: add changeset for use-on-cd shell fixes
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* feat: support x64-glibc-217

* add changeset

---------

Co-authored-by: Gal Schlezinger <gal@spitfire.co.il>
* fix(use): clarify interactive install prompt source

* chore: add changeset for prompt clarification
* perf(install): pin shell in generated env setup

* ci: pin docker platform for ARM installer tests

* chore: add changeset for installer shell update
@sourcery-ai

sourcery-ai Bot commented Apr 5, 2026

Copy link
Copy Markdown

Reviewer's Guide

Implements immediate application of the current directory Node version during fnm env --use-on-cd, improves shell hooks (especially zsh and Windows CMD), prefers explicit shell selection in installer-generated env setup, adds support for the x64-glibc-217 architecture, and updates tests, docs, CI, and versioning metadata for the 1.39.0 release.

Sequence diagram for fnm env --use-on-cd applying current directory version

sequenceDiagram
    actor User
    participant Shell
    participant FnmEnv as Env_command
    participant SetPath as set_path_for_multishell
    participant UseCmd as Use_command

    User->>Shell: runs startup script
    Shell->>FnmEnv: fnm env --use-on-cd --shell <shell>
    FnmEnv->>SetPath: set_path_for_multishell(multishell_path)
    SetPath-->>FnmEnv: PATH updated with multishell_path
    FnmEnv->>FnmEnv: config_with_multishell = config.clone().with_multishell_path(multishell_path)
    FnmEnv->>UseCmd: construct Use { version: None, install_if_missing: false, silent_if_unchanged: true, info_to_stderr: true }
    FnmEnv->>UseCmd: apply(config_with_multishell)
    UseCmd-->>FnmEnv: apply result (ignored on error)
    FnmEnv-->>Shell: shell.use_on_cd(config) script
    Shell-->>User: environment ready, current directory version already active
Loading

Class diagram for updated Use, FnmConfig, Arch, and Directories types

classDiagram
    class Use {
        +UserVersion~Option~ version
        +bool install_if_missing
        +bool silent_if_unchanged
        +bool info_to_stderr
        +apply(config: FnmConfig) Result
    }

    class FnmConfig {
        +Url node_dist_mirror
        +Url node_rc_mirror
        +Option~PathBuf~ base_dir
        +Option~PathBuf~ multishell_path
        +with_base_dir(base_dir: Option~PathBuf~) FnmConfig
        +with_multishell_path(multishell_path: PathBuf) FnmConfig
    }

    class Directories {
        <<struct>>
    }

    class Arch {
        <<enum>>
        X86
        X64
        X64Musl
        X64Glibc217
        Arm64
        Armv7l
        Ppc64le
        Aarch64AppleDarwin
        +as_str() &str
        +from_str(s: &str) Result~Arch~
    }

    FnmConfig --> Directories : uses
    FnmConfig --> Arch : configured_with
    Use --> FnmConfig : apply(config)

    class WindowsCmd {
        +use_on_cd(config: FnmConfig) Result~String~
    }

    class Zsh {
        +use_on_cd(config: FnmConfig) Result~String~
    }

    WindowsCmd ..> FnmConfig : generates cd macro
    Zsh ..> FnmConfig : generates chpwd hook
Loading

File-Level Changes

Change Details Files
Apply current directory version eagerly when running fnm env --use-on-cd and route its informational output to stderr when used internally.
  • Introduce set_path_for_multishell to prepend the multishell path (or its bin subdir) to PATH before running Use during env setup.
  • Clone and extend FnmConfig with a new with_multishell_path helper and use it to configure the internal Use command inside Env when use_on_cd is enabled.
  • Add an info_to_stderr flag to the Use command and route messages to stderr when invoked internally from fnm env, while keeping stdout clean for shell evaluation.
  • Update all internal Use invocations (install, install_new_version, etc.) to set info_to_stderr appropriately and tweak the interactive missing-version error prefix to include fnm.
  • Add end-to-end tests to verify that use-on-cd applies the version immediately after env setup and remains correct when env is sourced multiple times.
src/commands/env.rs
src/commands/use.rs
src/commands/install.rs
src/config.rs
src/directories.rs
e2e/use-on-cd.test.ts
Harden and test shell hooks for --use-on-cd, particularly for zsh and Windows CMD, and slightly simplify other shell hooks.
  • Change the zsh use_on_cd hook to remove any existing _fnm_autoload_hook before adding it again via add-zsh-hook -D chpwd _fnm_autoload_hook, and add a unit test asserting this behavior.
  • Wrap the Windows CMD doskey macro path in quotes to safely handle paths with spaces and add a test that exercises a base directory containing spaces.
  • Update cd.cmd to call cd /d so drive changes work correctly on Windows, while still running the fnm use logic based on FNM_VERSION_FILE_STRATEGY.
  • Remove redundant initial autoload invocations from the Fish, Bash, and PowerShell use_on_cd hooks so that version switching is driven purely by the cd alias or hook logic.
src/shell/zsh.rs
src/shell/windows_cmd/mod.rs
src/shell/windows_cmd/cd.cmd
src/shell/fish.rs
src/shell/bash.rs
src/shell/powershell.rs
Prefer explicit shell flags in installer-generated setup, document this guidance, and tweak CLI/docs examples.
  • Update .ci/install.sh to call fnm env with explicit --shell flags for zsh, fish, and bash in the generated shell configuration blocks.
  • Adjust CLI help text and docs (README.md, docs/commands.md) to show examples that pass explicit --shell values instead of relying on runtime shell inference.
  • Add a tip to the README recommending explicit shell selection to reduce startup overhead and avoid process tree detection.
.ci/install.sh
src/cli.rs
README.md
docs/commands.md
Extend architecture support with x64-glibc-217 and wire it through parsing, display, and release notes.
  • Add a new X64Glibc217 variant to the Arch enum plus corresponding display string mapping and FromStr parsing for x64-glibc-217.
  • Introduce a changeset entry documenting --arch x64-glibc-217 support for fnm env.
  • Ensure new arch participation by including it in the release metadata (version bump and changelog).
src/arch.rs
.changeset/brave-timers-move.md
CHANGELOG.md
Cargo.toml
package.json
Refine CI for the installer script and clean up YAML/style issues.
  • Update the installation_script workflow to use a matrix with docker_platform so ARM Docker runs specify the correct --platform flag per image.
  • Normalize YAML formatting (quotes, spacing) and tweak job steps to use the new matrix fields while keeping behavior equivalent for non-ARM cases.
  • Add a changeset entry describing the installer CI platform selection fix and explicit shell preference.
  • Remove obsolete changeset files that have been folded into the 1.39.0 release.
.github/workflows/installation_script.yml
.changeset/cuddly-cars-ring.md
.changeset/fair-carrots-greet.md
.changeset/grumpy-dingos-turn.md
.changeset/odd-mayflies-poke.md
.changeset/twelve-badgers-happen.md
Prepare and document the 1.39.0 release, including behavior tweaks already implemented in code.
  • Bump crate and package versions from 1.38.1 to 1.39.0 in Rust and npm metadata.
  • Add a 1.39.0 section to the changelog describing the new env --use-on-cd behavior, use flag on install, zsh/CMD hook robustness, default-version behavior, and Rust toolchain bump.
  • Perform minor doc and SVG touchups associated with the release branding.
CHANGELOG.md
Cargo.toml
package.json
docs/fnm.svg

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@snyk-io

snyk-io Bot commented Apr 5, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@Dargon789 Dargon789 enabled auto-merge (squash) April 5, 2026 12:09

@sourcery-ai sourcery-ai 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.

Hey - I've found 2 issues, and left some high level feedback:

  • In set_path_for_multishell, the unsafe block around std::env::set_var is unnecessary since set_var is safe; you can drop the unsafe entirely.
  • When prepending multishell_path to PATH in set_path_for_multishell, consider de-duplicating the entry so repeated fnm env --use-on-cd calls don’t keep growing PATH with identical prefixes.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `set_path_for_multishell`, the `unsafe` block around `std::env::set_var` is unnecessary since `set_var` is safe; you can drop the `unsafe` entirely.
- When prepending `multishell_path` to `PATH` in `set_path_for_multishell`, consider de-duplicating the entry so repeated `fnm env --use-on-cd` calls don’t keep growing `PATH` with identical prefixes.

## Individual Comments

### Comment 1
<location path="CHANGELOG.md" line_range="13" />
<code_context>
+
+### Patch Changes
+
+- [#1268](https://github.com/Schniz/fnm/pull/1268) [`ddab191`](https://github.com/Schniz/fnm/commit/ddab1911dc33a24aeec9d2b8139df1916c518d64) Thanks [@isaacl](https://github.com/isaacl)! - Make `fnm default` with no trailing positional argument to return the default version
+
+- [#1517](https://github.com/Schniz/fnm/pull/1517) [`d3747af`](https://github.com/Schniz/fnm/commit/d3747affd604223c752171c14944bd5773096b09) Thanks [@Schniz](https://github.com/Schniz)! - Improve `--use-on-cd` shell hook robustness by de-duplicating the zsh hook on re-source and fixing Windows CMD hook behavior for paths with spaces and drive changes.
</code_context>
<issue_to_address>
**nitpick (typo):** Tighten grammar in the `fnm default` description.

The phrase "Make `fnm default` with no trailing positional argument to return the default version" is a bit awkward. Consider wording it as "Make `fnm default` with no trailing positional argument return the default version" or "Have `fnm default` with no trailing positional argument return the default version."

```suggestion
- [#1268](https://github.com/Schniz/fnm/pull/1268) [`ddab191`](https://github.com/Schniz/fnm/commit/ddab1911dc33a24aeec9d2b8139df1916c518d64) Thanks [@isaacl](https://github.com/isaacl)! - Have `fnm default` with no trailing positional argument return the default version
```
</issue_to_address>

### Comment 2
<location path="docs/commands.md" line_range="383" />
<code_context>
 This command generates a series of shell commands that should be evaluated by your shell to create a fnm-ready environment.

-Each shell has its own syntax of evaluating a dynamic expression. For example, evaluating fnm on Bash and Zsh would look like `eval "$(fnm env)"`. In Fish, evaluating would look like `fnm env | source`
+Each shell has its own syntax of evaluating a dynamic expression. For example, evaluating fnm on Bash and Zsh would look like `eval "$(fnm env --shell bash)"`. In Fish, evaluating would look like `fnm env --shell fish | source`

 Usage: fnm env [OPTIONS]
</code_context>
<issue_to_address>
**nitpick (typo):** Add terminal punctuation to the modified sentence for consistency.

The sentence ending with `fnm env --shell fish | source` is the only one in the paragraph without a period. Please add a period for consistency.

```suggestion
Each shell has its own syntax of evaluating a dynamic expression. For example, evaluating fnm on Bash and Zsh would look like `eval "$(fnm env --shell bash)"`. In Fish, evaluating would look like `fnm env --shell fish | source`.
```
</issue_to_address>

Fix all in Cursor


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread CHANGELOG.md
Comment thread docs/commands.md

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

Code Review

This pull request bumps fnm to version 1.39.0 and introduces several performance improvements and bug fixes. A significant change is the reduction of shell startup overhead for env --use-on-cd by applying the initial Node version internally during the env command execution, rather than relying on a subsequent shell hook. The PR also adds support for the x64-glibc-217 architecture, improves the robustness of shell hooks (specifically for Zsh and Windows CMD), and updates documentation to prefer explicit shell flags. Feedback was provided regarding the use of the Error log level to redirect informational messages to stderr, as this might lead to misleading error-style formatting for users.

Comment thread src/commands/use.rs
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
auto-merge was automatically disabled April 17, 2026 22:23

Head branch was pushed to by a user without write access

theborowski and others added 3 commits April 17, 2026 18:24
* feat: add possible values to the arch help docs.

* add a changeset

---------

Co-authored-by: Gal Schlezinger <gal@spitfire.co.il>
* docs: add e2e testing guide for agents

* docs: trim AGENTS e2e section

* docs: refine AGENTS guidance for e2e and changesets
* Add random number to multishell symlinks

This protects against collisions when multiple shells are started in parallel
inside Bubblewrap sandboxes, as each shell will have the same timestamp and the
same low PID (e.g. 2), since Bubblewrap sandboxes have isolated PID spaces.

* Add a changeset

---------

Co-authored-by: Gal Schlezinger <gal@spitfire.co.il>
@vercel

vercel Bot commented Apr 17, 2026

Copy link
Copy Markdown

@joliss is attempting to deploy a commit to the Foundry development Team on Vercel.

A member of the Team first needs to authorize it.

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.

fnm env --use-on-cd applying current directory version

5 participants