Skip to content

Conversation

@LesnyRumcajs
Copy link
Member

@LesnyRumcajs LesnyRumcajs commented Dec 18, 2025

Summary of changes

Changes introduced in this pull request:

  • moved from Makefile to Mise http://mise.jdx.dev/ and ported most commands
  • I'll add profiling to docs in a separate PR.

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Summary by CodeRabbit

  • Chores

    • Migrated CI, Docker, and local workflows to a unified task-runner and action wrapper; removed legacy Makefile and related make-based orchestration.
  • New Features

    • Added centralized task configuration exposing install, lint, format, test, and coverage tasks; added an installer script used in images.
  • Documentation

    • Updated README, contributing, and getting-started docs and changelog to reference the new task-runner commands.
  • Bug Fixes

    • Ensured the installer script is included in Docker build context.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 18, 2025

Walkthrough

Replaces Makefile-driven build/lint/test flows with a mise task runner: deletes Makefile, adds mise.toml and supporting scripts, updates Dockerfile and .dockerignore to install mise, and converts GitHub Actions workflows to use jdx/mise-action@v3 and mise commands; docs updated accordingly. (47 words)

Changes

Cohort / File(s) Summary
CI Workflows
​.github/workflows/...
\ .github/workflows/butterflynet.yml`, ` .github/workflows/cargo-advisories.yml`, ` .github/workflows/coverage.yml`, ` .github/workflows/docker.yml`, ` .github/workflows/forest.yml`, ` .github/workflows/go-lint.yml`, ` .github/workflows/rust-lint.yml`, ` .github/workflows/scripts-lint.yml`, ` .github/workflows/unit-tests.yml``
Replaced actions/setup-go@v6 and many Makefile run commands with uses: jdx/mise-action@v3 and mise task invocations; adjusted some triggers/filters to reference mise.toml and consolidated lint/test steps to use mise.
Build / Container / Ignore
\Makefile`, `mise.toml`, `.dockerignore`, `Dockerfile``
Deleted Makefile. Added mise.toml. Updated .dockerignore to un-ignore scripts/install_mise.sh (!scripts/install_mise.sh). Dockerfile runs ./scripts/install_mise.sh and uses mise trust / mise run install in place of make install.
Installer & Task Scripts
\scripts/install_mise.sh`, `mise-tasks/add_license.sh`, `mise-tasks/lint/lists.sh``
Added GPG-verified installer scripts/install_mise.sh. Converted a comment to MISE metadata in mise-tasks/add_license.sh. Added mise-tasks/lint/lists.sh to sort/uniq list files.
Task Runner Config
\mise.toml``
New mise.toml exposing public tasks for install, install-lint-tools, many lint:* tasks, fmt, test (docs + Rust), codecov, clean, and declared tool versions.
Linters / Utilities
\scripts/linters/find_unused_deps.rb``
Narrowed manifest discovery glob from **/*.toml to **/Cargo.toml for unused-deps detection.
Docs & Minor Files
\README.md`, `CONTRIBUTING.md`, `CHANGELOG.md`, `docs/dictionary.txt`, `docs/docs/users/getting_started/install.md`, `build/vendored-docs-redirect.index.html``
Replaced make instructions with mise equivalents, added "mise-en-place" dictionary entry, updated changelog and install docs, and minor HTML/text formatting changes.

Sequence Diagram(s)

sequenceDiagram
  actor Developer
  participant GitHub as GitHub Actions
  participant MiseAction as jdx/mise-action
  participant Runner as Job Runner
  participant Tools as Toolchain

  Developer->>GitHub: push PR / commit
  GitHub->>MiseAction: invoke mise-action step
  MiseAction->>Runner: provision env & install mise
  Runner->>Tools: run mise tasks (lint / test / install / codecov)
  Tools-->>Runner: produce outputs/artifacts
  Runner-->>GitHub: upload artifacts / report status
Loading
sequenceDiagram
  participant Docker as Docker build
  participant Script as scripts/install_mise.sh
  participant GPG as Keyserver/GPG
  participant Mise as mise installer
  participant Tools as Toolchain

  Docker->>Script: RUN ./scripts/install_mise.sh
  Script->>GPG: import release key & verify signature
  Script->>Mise: execute verified installer script
  Mise-->>Docker: mise installed
  Docker->>Mise: RUN mise trust && mise run install
  Mise->>Tools: install project binaries
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • akaladarshi
  • hanabi1224
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: migrating the build system from Makefile to Mise, which is the primary objective of this PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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


📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4d384f and 46f7ba8.

📒 Files selected for processing (5)
  • .dockerignore
  • .github/workflows/go-lint.yml
  • README.md
  • docs/dictionary.txt
  • mise.toml
🚧 Files skipped from review as they are similar to previous changes (3)
  • .github/workflows/go-lint.yml
  • README.md
  • mise.toml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Build MacOS
  • GitHub Check: Build Ubuntu
  • GitHub Check: cargo-publish-dry-run
  • GitHub Check: All lint checks
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: Coverage
  • GitHub Check: tests-release
  • GitHub Check: rubocop
🔇 Additional comments (2)
.dockerignore (1)

17-17: LGTM!

The negated pattern correctly creates an exception to include scripts/install_mise.sh in the Docker build context, allowing the Dockerfile to execute the new mise installation script despite the broader scripts/ exclusion on line 16.

docs/dictionary.txt (1)

96-96: Verify the dictionary entry matches primary terminology in the codebase.

The entry "mise-en-place" is correctly alphabetized, but since the PR introduces the "mise" task runner (not the French phrase), verify whether the spell-check dictionary should contain "mise" instead of (or in addition to) "mise-en-place". The tool name "mise" is what will appear in code, configs, and most documentation.


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

@LesnyRumcajs LesnyRumcajs force-pushed the use-mise branch 6 times, most recently from f11e87e to 983d358 Compare December 19, 2025 10:58
@LesnyRumcajs LesnyRumcajs changed the title chore: move from Makefile to mise [skip ci] chore: move from Makefile to mise Jan 8, 2026
@LesnyRumcajs LesnyRumcajs force-pushed the use-mise branch 3 times, most recently from 42d496b to 19b0dac Compare January 8, 2026 11:45
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @scripts/install_mise.sh:
- Line 10: Update the curl invocation that downloads the signature in the
scripts/install_mise.sh script to fail on HTTP errors and surface errors
cleanly: modify the curl call (the line starting with "curl
https://mise.jdx.dev/install.sh.sig | gpg --decrypt > install-mise.sh") to
include --fail and add --silent --show-error (or at least --fail --show-error)
so HTTP error responses aren't piped into gpg and error output is clearer.
- Around line 5-9: The gpg command hardcodes the fingerprint instead of using
the GPG_KEY variable; replace the literal fingerprint in the gpg --recv-keys
invocation with the variable (e.g., use "$GPG_KEY" or "${GPG_KEY}") so the
script uses the exported GPG_KEY value, and ensure it is quoted to avoid
word-splitting.
🧹 Nitpick comments (1)
mise.toml (1)

174-177: Consider pinning tool versions for reproducibility.

The tools are configured with "latest" which can lead to non-reproducible builds and potential breakage from version changes. Consider pinning to specific versions (e.g., go = "1.25", yarn = "4.x") to ensure consistent behavior across environments.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab4982b and 16762a0.

📒 Files selected for processing (6)
  • .dockerignore
  • CHANGELOG.md
  • Dockerfile
  • build/vendored-docs-redirect.index.html
  • mise.toml
  • scripts/install_mise.sh
🚧 Files skipped from review as they are similar to previous changes (2)
  • CHANGELOG.md
  • build/vendored-docs-redirect.index.html
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.

Applied to files:

  • mise.toml
📚 Learning: 2025-08-07T13:39:29.732Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:58-58
Timestamp: 2025-08-07T13:39:29.732Z
Learning: In the Forest project, auto-generated files (like those generated by rust2go tool) should be skipped during code review as they are not meant to be manually edited.

Applied to files:

  • mise.toml
📚 Learning: 2025-10-17T09:36:15.757Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.

Applied to files:

  • Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Coverage
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: Check
  • GitHub Check: Deploy to Cloudflare Pages
  • GitHub Check: rubocop
  • GitHub Check: All lint checks
  • GitHub Check: tests-release
  • GitHub Check: cargo-publish-dry-run
  • GitHub Check: Build MacOS
  • GitHub Check: Build Ubuntu
🔇 Additional comments (1)
scripts/install_mise.sh (1)

5-12: The original review comment incorrectly questions whether MISE_VERSION and MISE_INSTALL_PATH are used by the installer. Both environment variables are officially supported by the mise installer script and are used during installation:

  • MISE_VERSION pins which mise release to install
  • MISE_INSTALL_PATH sets where the mise binary will be written

These variables are correctly exported in the script and should not be removed.

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @mise.toml:
- Around line 10-17: Initialize SLIM_FLAGS to an empty string before the
conditional and quote variable expansions in the run script: set SLIM_FLAGS=""
before testing usage_slim, keep the conditional that sets SLIM_FLAGS when
usage_slim is true, and change unquoted ${usage_profile} and ${SLIM_FLAGS} to
quoted expansions so the cargo install invocation uses "${usage_profile}" and
"${SLIM_FLAGS}" to avoid unbound-variable and word-splitting issues.
- Around line 174-177: The tools table currently pins all tool versions to
"latest", which makes builds non-reproducible; update the [tools] section by
replacing the "latest" values for the keys cargo-binstall, go, and yarn with
fixed version strings (e.g., cargo-binstall -> "1.16.6", go -> "1.25.5", yarn ->
"4.12.0") or alternatively generate and commit a mise.lock to lock resolved
versions so CI and developer environments use identical tool versions.
🧹 Nitpick comments (4)
mise.toml (4)

61-67: Consider optimizing gem installation.

The task installs the toml-rb gem on every invocation, which is inefficient. Consider checking if the gem is already installed or documenting that users should install it separately.

♻️ Proposed optimization
 run = '''
-gem install --no-document toml-rb
+gem list -i toml-rb >/dev/null 2>&1 || gem install --no-document toml-rb
 ruby scripts/linters/find_unused_deps.rb
 '''

102-104: Consider including lint:toml in the Rust linting tasks.

Since TOML files (like Cargo.toml) are integral to Rust projects, you may want to include lint:toml in the lint:all-rust task for completeness.

♻️ Proposed enhancement
 [tasks."lint:all-rust"]
 description = "Run all Rust linting tasks."
-depends = ["lint:deny", "lint:spellcheck", "lint:rust-fmt", "lint:clippy", "lint:unused-deps"]
+depends = ["lint:deny", "lint:spellcheck", "lint:toml", "lint:rust-fmt", "lint:clippy", "lint:unused-deps"]

106-122: Simplify yarn initialization.

Both tasks run yarn && yarn install, which is redundant since yarn (with no arguments) already performs yarn install. This can be simplified to just yarn install.

♻️ Proposed simplification

For lint:yaml:

 run = '''
-corepack enable && yarn && yarn install
+corepack enable && yarn install
 yarn yaml-fmt
 yarn yaml-check
 '''

For fmt:

 run = '''
 cargo fmt --all
 taplo fmt
-corepack enable && yarn && yarn install
+corepack enable && yarn install
 yarn md-fmt
 yarn yaml-fmt
 '''

156-166: Consider using mise's task orchestration syntax instead of shell invocations.

While mise supports calling mise task run from within tasks, the idiomatic approach is to use inline task objects. The code can be refactored to:

run = [
  { task = "test:rust", args = ["${usage_profile?}"] },
  { task = "test:docs", args = ["${usage_profile?}"] }
]

This is more aligned with mise's task orchestration patterns and provides better clarity around task dependencies and parameter passing.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 664739d and 911d87d.

📒 Files selected for processing (1)
  • mise.toml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.

Applied to files:

  • mise.toml
📚 Learning: 2025-08-07T13:39:29.732Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:58-58
Timestamp: 2025-08-07T13:39:29.732Z
Learning: In the Forest project, auto-generated files (like those generated by rust2go tool) should be skipped during code review as they are not meant to be manually edited.

Applied to files:

  • mise.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Build Ubuntu
  • GitHub Check: Build MacOS
  • GitHub Check: cargo-publish-dry-run
  • GitHub Check: tests-release
  • GitHub Check: All lint checks
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: rubocop
  • GitHub Check: Coverage
🔇 Additional comments (6)
mise.toml (6)

20-23: LGTM!

The task correctly installs the required linting tools using cargo-binstall.


50-59: LGTM!

The task comprehensively checks Clippy across different feature combinations and the docs.rs build configuration.


69-96: LGTM!

All specialized linting tasks are properly configured with their respective tools and targets.


124-129: LGTM!

The cleanup task safely removes build artifacts and dependencies.


168-172: LGTM!

The code coverage task is correctly configured.


37-42: Verify error propagation in mise task run blocks.

The task runs two taplo commands sequentially without explicit error handling. Confirm whether mise's run block execution automatically fails on the first command's error (e.g., if taplo fmt --check fails) or if both commands execute regardless. If explicit error handling is needed, use the && operator.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
mise.toml (4)

10-17: Initialize SLIM_FLAGS to prevent potential shell errors.

The SLIM_FLAGS variable is only set when usage_slim is true, but it's always referenced on line 16. While bash tolerates unset variables in unquoted expansions, this is fragile and could fail with set -u or in stricter shell environments.

🛡️ Defensive coding improvement
 run = '''
 echo "Installing Forest binaries with profile: ${usage_profile?}"
 [[ -n "${usage_verbose}" ]] && set -x
+SLIM_FLAGS=""
 if [ "${usage_slim}" = true ]; then
     SLIM_FLAGS="--no-default-features --features slim"
 fi
 cargo install --profile ${usage_profile} ${SLIM_FLAGS} --locked --path . --force
 '''

106-112: Remove redundant yarn command.

Line 109 runs yarn && yarn install, but in Yarn v2+ (Berry), yarn without arguments is equivalent to yarn install. The first invocation is redundant.

🔧 Simplify command
 run = '''
-corepack enable && yarn && yarn install
+corepack enable && yarn install
 yarn yaml-fmt
 yarn yaml-check
 '''

114-122: Remove redundant yarn command.

Line 119 has the same redundancy as tasks."lint:yaml" - running yarn && yarn install when yarn install alone is sufficient in Yarn v2+.

🔧 Simplify command
 run = '''
 cargo fmt --all
 taplo fmt
-corepack enable && yarn && yarn install
+corepack enable && yarn install
 yarn md-fmt
 yarn yaml-fmt
 '''

174-177: Pin tool versions for reproducible builds.

All tools are set to "latest", which can cause CI builds to break unexpectedly when tools update and may introduce inconsistencies across different build environments. For production CI/CD pipelines, pinning to specific versions ensures deterministic, reproducible builds.

📌 Pin tool versions

Consider pinning to specific versions:

 [tools]
-cargo-binstall = "latest"
-go = "latest"
-yarn = "latest"
+cargo-binstall = "1.10.15"  # or your preferred version
+go = "1.23.4"                # or your preferred version  
+yarn = "4.5.3"               # or your preferred version

After determining appropriate versions, you can periodically update them in a controlled manner through dependency update PRs.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 911d87d and 5df7e32.

📒 Files selected for processing (1)
  • mise.toml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.

Applied to files:

  • mise.toml
📚 Learning: 2025-08-07T13:39:29.732Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:58-58
Timestamp: 2025-08-07T13:39:29.732Z
Learning: In the Forest project, auto-generated files (like those generated by rust2go tool) should be skipped during code review as they are not meant to be manually edited.

Applied to files:

  • mise.toml
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest pins Rust toolchain to 1.89.0 via rust-toolchain.toml; features stabilized in 1.88 (e.g., let-chains) are acceptable in this codebase.

Applied to files:

  • mise.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Build Ubuntu
  • GitHub Check: cargo-publish-dry-run
  • GitHub Check: Build MacOS
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: tests-release
  • GitHub Check: Coverage
  • GitHub Check: All lint checks
  • GitHub Check: Go lint checks
  • GitHub Check: rubocop
  • GitHub Check: Check
  • GitHub Check: Deploy to Cloudflare Pages
🔇 Additional comments (1)
mise.toml (1)

168-172: Verify codecov scope is intentional.

The codecov task only generates coverage for the forest-filecoin package. The workspace also contains forest-interop-tests (the interop-tests member), which is not included in the coverage report. Confirm whether coverage for forest-interop-tests should be added or if excluding it is intentional.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @mise.toml:
- Around line 76-81: The shellcheck task's run script uses the **/*.sh glob
which requires Bash's globstar option; update the tasks."lint:shellcheck" run
command to enable globstar before invoking shellcheck (e.g., set shopt -s
globstar or run bash with -O globstar) so nested scripts are matched, or
alternatively replace the glob with a find/xargs pipeline that locates all *.sh
under SCRIPTDIR and feeds them to shellcheck; ensure you reference the existing
run string and SCRIPTDIR symbol when making the change.
🧹 Nitpick comments (3)
mise.toml (3)

106-112: Remove redundant yarn command.

Line 109 runs yarn && yarn install, but yarn without arguments defaults to yarn install, making the second call redundant.

♻️ Proposed refactor
 [tasks."lint:yaml"]
 description = "Lint YAML files."
 run = '''
-corepack enable && yarn && yarn install
+corepack enable && yarn install
 yarn yaml-fmt
 yarn yaml-check
 '''

114-122: Remove redundant yarn command.

Same redundancy as in lint:yaml: yarn && yarn install on line 119 can be simplified to just yarn install.

♻️ Proposed refactor
 [tasks.fmt]
 description = "Format all supported code."
 run = '''
 cargo fmt --all
 taplo fmt
-corepack enable && yarn && yarn install
+corepack enable && yarn install
 yarn md-fmt
 yarn yaml-fmt
 '''

61-67: Consider caching gem installation for efficiency.

The task installs toml-rb gem on every run, which is inefficient but functional. Consider documenting that users should pre-install the gem, or check if it's already installed before attempting installation.

♻️ Proposed optimization
 [tasks."lint:unused-deps"]
 description = "Check for unused dependencies in Rust code."
 tools.ruby = "latest"
 run = '''
-gem install --no-document toml-rb
+gem list toml-rb -i || gem install --no-document toml-rb
 ruby scripts/linters/find_unused_deps.rb
 '''
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5df7e32 and f4d384f.

📒 Files selected for processing (1)
  • mise.toml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.

Applied to files:

  • mise.toml
📚 Learning: 2025-08-07T13:39:29.732Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:58-58
Timestamp: 2025-08-07T13:39:29.732Z
Learning: In the Forest project, auto-generated files (like those generated by rust2go tool) should be skipped during code review as they are not meant to be manually edited.

Applied to files:

  • mise.toml
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest pins Rust toolchain to 1.89.0 via rust-toolchain.toml; features stabilized in 1.88 (e.g., let-chains) are acceptable in this codebase.

Applied to files:

  • mise.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Build MacOS
  • GitHub Check: Build Ubuntu
  • GitHub Check: cargo-publish-dry-run
  • GitHub Check: tests-release
  • GitHub Check: Coverage
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: All lint checks
  • GitHub Check: rubocop
  • GitHub Check: Check
  • GitHub Check: Deploy to Cloudflare Pages
🔇 Additional comments (4)
mise.toml (4)

131-166: LGTM!

The test task structure is well-organized with proper profile argument handling and task composition. The separation of doctests and regular tests via nextest is appropriate given nextest's limitation with doctests.


13-13: No issue found. The comparison [ "${usage_slim}" = true ] is correct—mise passes boolean flags as the literal strings "true" or "false" to task environment variables, so the check works as intended.


168-172: No action required — the codecov command is already proven to work.

This task duplicates the existing Makefile target codecov, which uses the identical command and is already running successfully in the CI pipeline (.github/workflows/coverage.yml). Both cargo-llvm-cov availability and the -p forest-filecoin package scope are already validated by the active CI workflow. If adding this task to mise.toml for convenience, ensure it remains synchronized with the Makefile version.

Likely an incorrect or invalid review comment.


174-177: All tool versions are valid. The Go version 1.25.5, Yarn 4.12, and cargo-binstall 1.16.6 are all available releases as of January 2026. No version corrections needed.

Comment on lines +76 to +81
[tasks."lint:shellcheck"]
description = "Lint shell scripts using shellcheck."
tools.shellcheck = "latest"
run = '''
shellcheck --external-sources --source-path=SCRIPTDIR **/*.sh
'''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Enable globstar for recursive shell script matching.

The **/*.sh pattern requires bash's globstar option to expand recursively. Without it, shellcheck will only match files in the current directory, potentially missing nested scripts.

🐚 Proposed fix to enable globstar
 [tasks."lint:shellcheck"]
 description = "Lint shell scripts using shellcheck."
 tools.shellcheck = "latest"
 run = '''
+shopt -s globstar
 shellcheck --external-sources --source-path=SCRIPTDIR **/*.sh
 '''
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[tasks."lint:shellcheck"]
description = "Lint shell scripts using shellcheck."
tools.shellcheck = "latest"
run = '''
shellcheck --external-sources --source-path=SCRIPTDIR **/*.sh
'''
[tasks."lint:shellcheck"]
description = "Lint shell scripts using shellcheck."
tools.shellcheck = "latest"
run = '''
shopt -s globstar
shellcheck --external-sources --source-path=SCRIPTDIR **/*.sh
'''
🤖 Prompt for AI Agents
In @mise.toml around lines 76 - 81, The shellcheck task's run script uses the
**/*.sh glob which requires Bash's globstar option; update the
tasks."lint:shellcheck" run command to enable globstar before invoking
shellcheck (e.g., set shopt -s globstar or run bash with -O globstar) so nested
scripts are matched, or alternatively replace the glob with a find/xargs
pipeline that locates all *.sh under SCRIPTDIR and feeds them to shellcheck;
ensure you reference the existing run string and SCRIPTDIR symbol when making
the change.

@codecov
Copy link

codecov bot commented Jan 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.51%. Comparing base (e47b875) to head (46f7ba8).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

see 7 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2bd2f58...46f7ba8. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@LesnyRumcajs LesnyRumcajs enabled auto-merge January 9, 2026 10:02
@LesnyRumcajs LesnyRumcajs added this pull request to the merge queue Jan 9, 2026
Merged via the queue into main with commit 415224f Jan 9, 2026
53 checks passed
@LesnyRumcajs LesnyRumcajs deleted the use-mise branch January 9, 2026 11:04
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.

3 participants