Skip to content

1462#103

Merged
Dargon789 merged 8 commits into
masterfrom
1462
Jan 26, 2026
Merged

1462#103
Dargon789 merged 8 commits into
masterfrom
1462

Conversation

@Dargon789

@Dargon789 Dargon789 commented Jan 26, 2026

Copy link
Copy Markdown
Owner

Summary by Sourcery

Improve the fnm installation script’s portability and robustness while updating a Rust dependency version.

Bug Fixes:

  • Fix install script incompatibilities with POSIX /bin/sh by avoiding bash-specific syntax and utilities.
  • Ensure temporary download directories are always cleaned up and handle failures to create them gracefully.

Enhancements:

  • Add a portable temporary directory creation helper and use more robust command detection in the install script.
  • Simplify shell configuration sourcing guidance and modernize fnm env evaluation in generated shell config.

Chores:

  • Bump the indoc Rust crate dependency to version 2.0.7.

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@vercel

vercel Bot commented Jan 26, 2026

Copy link
Copy Markdown

Deployment failed with the following error:

Resource is limited - try again in 0 ms (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/dargon789-forge?upgradeToPro=build-rate-limit

@sourcery-ai

sourcery-ai Bot commented Jan 26, 2026

Copy link
Copy Markdown

Reviewer's Guide

Make the CI install script more portable and POSIX-compliant, harden its temp-dir handling and cleanup, improve dependency checks, and adjust shell integration commands, plus add a changeset for a Rust dependency bump.

Sequence diagram for hardened temp directory handling in download_fnm

sequenceDiagram
  actor CI as CI_or_User
  participant Install as install_sh
  participant Mktempdir as mktempdir
  participant Curl as curl
  participant Unzip as unzip
  participant FS as Filesystem

  CI->>Install: run install.sh
  Install->>Mktempdir: DOWNLOAD_DIR=$(mktempdir)
  alt mktempdir succeeds
    Mktempdir-->>Install: path to temp directory
    Install->>Install: trap "rm -rf '$DOWNLOAD_DIR'" EXIT INT HUP TERM
    Install->>FS: mkdir -p INSTALL_DIR
    Install->>Curl: download archive to DOWNLOAD_DIR/FILENAME.zip
    alt curl succeeds
      Curl-->>Install: archive file
      Install->>Unzip: unzip archive into DOWNLOAD_DIR
      Unzip-->>Install: extracted binary
      Install->>FS: mv binary to INSTALL_DIR
    else curl fails
      Curl-->>Install: error
      Install->>CI: print download failed message
      Install-->>CI: exit 1 (trap removes DOWNLOAD_DIR)
    end
  else mktempdir fails
    Mktempdir-->>Install: error
    Install->>CI: print unable to create temp directory
    Install-->>CI: exit 1
  end
  Install-->>CI: script exits (trap removes DOWNLOAD_DIR)
Loading

Flow diagram for updated CI install.sh logic

flowchart TD
  Start([Start]) --> ParseArgs["parse_args: read flags
--use-homebrew, --force-install,
--skip-shell, release, install dir"]

  ParseArgs --> DetectOS["Detect OS and architecture"]
  DetectOS --> CheckDeps["check_dependencies:
check curl, unzip,
optionally brew"]

  CheckDeps -->|missing deps and not FORCE_INSTALL| ExitDeps["Print missing dependency message
and exit"]
  CheckDeps -->|deps ok or FORCE_INSTALL| SetFilename["set_filename based on OS and arch"]

  SetFilename --> DecideHB{USE_HOMEBREW == true?}

  DecideHB -->|yes| InstallHB["brew install fnm"]
  DecideHB -->|no| DownloadFnm["download_fnm:
create temp dir via mktempdir,
set trap for cleanup,
mkdir -p INSTALL_DIR,
use curl and unzip,
move binary into INSTALL_DIR"]

  InstallHB --> AfterInstall
  DownloadFnm --> AfterInstall["fnm binary installed"]

  AfterInstall --> SkipShell{SKIP_SHELL == true?}

  SkipShell -->|yes| EndNoShell([End: installation only])
  SkipShell -->|no| SetupShell["setup_shell:
append integration to shell rc file,
use eval $(fnm env),
print sourcing instructions"]

  SetupShell --> End([End: installation + shell integration])
Loading

File-Level Changes

Change Details Files
Improve shell script portability and POSIX compliance for the CI installer.
  • Switch script shebang from bash to sh and replace bash-specific constructs with POSIX-compliant ones
  • Initialize new global variables for configuration flags and state tracking
  • Use single-bracket test syntax and printf instead of echo -n for better portability
  • Replace hash with command -v for checking executable availability
.ci/install.sh
Harden download and temporary directory handling in the installer.
  • Introduce a portable mktempdir helper with multiple mktemp strategies and a secure mkdir fallback loop
  • Use mktempdir to create the download directory and fail fast with an error if it cannot be created
  • Add a trap to always clean up the temporary download directory on script exit or interruption
  • Silence mkdir output when creating the installation directory
.ci/install.sh
Adjust shell configuration setup logic for fnm environment initialization.
  • Switch from legacy backtick command substitution to $(...) when writing fnm env to shell config
  • Change the post-install sourcing instruction to use dot (.) for POSIX shells while keeping source for fish
.ci/install.sh
Fix local variable usage and directory creation in helper function.
  • Remove bash-specific local declaration in ensure_containing_dir_exists and use a lowercase variable name
  • Ensure parent directory creation messages and mkdir calls use the updated variable name
.ci/install.sh
Document a dependency update via changeset metadata.
  • Add a changeset describing a patch release for fnm due to updating the Rust crate indoc to v2.0.7
.changeset/perfect-beds-notice.md

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 Jan 26, 2026

Copy link
Copy Markdown

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

Status Scanner 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.

@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 1 issue, and left some high level feedback:

  • In ensure_containing_dir_exists, replacing the local variable with a global containing_dir risks unintended reuse elsewhere in the script; consider using a uniquely prefixed name or a subshell to keep the scope local.
  • The mktempdir fallback loop creates predictable directory names under ${TMPDIR:-/tmp}; if this script may run in multi-user or shared environments, consider adding a brief comment or extra randomness to clarify/mitigate the security implications of the fallback path.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `ensure_containing_dir_exists`, replacing the `local` variable with a global `containing_dir` risks unintended reuse elsewhere in the script; consider using a uniquely prefixed name or a subshell to keep the scope local.
- The `mktempdir` fallback loop creates predictable directory names under `${TMPDIR:-/tmp}`; if this script may run in multi-user or shared environments, consider adding a brief comment or extra randomness to clarify/mitigate the security implications of the fallback path.

## Individual Comments

### Comment 1
<location> `.ci/install.sh:173-174` </location>
<code_context>

-  echo -n "Checking availability of unzip... "
-  if hash unzip 2>/dev/null; then
+  printf "Checking availability of unzip... "
+  if command -v unzip >/dev/null 2>&1; then
     echo "OK!"
   else
</code_context>

<issue_to_address>
**question (bug_risk):** Unzip is required even when Homebrew is used, which may be stricter than necessary.

When `USE_HOMEBREW="true"`, the script installs via `brew install fnm` and never uses `unzip`, but still enforces its presence and may set `SHOULD_EXIT="true"`. This can abort the script in valid Homebrew environments without `unzip`. If that’s not intentional, consider skipping the `unzip` check when `USE_HOMEBREW=true` or only requiring it on the non-Homebrew path.
</issue_to_address>

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 .ci/install.sh
@Dargon789 Dargon789 merged commit 90b57d8 into master Jan 26, 2026
@Dargon789 Dargon789 linked an issue Jan 26, 2026 that may be closed by this pull request
This was referenced Mar 1, 2026
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.

[vc]: # sixty-rice-do.md

2 participants