fix(deps): run package managers through mise exec - #34
Conversation
A worktree is created by a separate process, so the shell hook that puts mise-managed tools on PATH never fires there. In a project whose toolchain is pinned by mise, `worktree create` / `worktree checkout <PR URL>` died with `dotnet: command not found` while installing dependencies. - install_deps prefixes commands with `mise exec --` when a mise config applies to the worktree, and runs them directly otherwise - find_mise_configs walks from the install directory up to the task directory, so a multi-repo config living in the task dir (symlinked from the project root) is found from a sub-repo; it never walks past that boundary into unrelated configs - configs are passed to `mise trust` first: a worktree is a brand new absolute path, so a config trusted in the project root is untrusted there and `mise exec` aborts instead of prompting in this non-interactive context. `.tool-versions` carries no executable content and is not trusted - WORKTREE_NO_MISE=1 opts out of mise entirely - run test/deps.bats in CI, which was never wired up Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughworktree作成・checkout時の依存関係インストールにmise対応を追加し、設定探索・trust・ Changesmise対応の依存関係インストール
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant WorktreeCommand
participant install_deps
participant mise
participant PackageManager
WorktreeCommand->>install_deps: worktree path と task directory を渡す
install_deps->>mise: 設定ファイルを探索
install_deps->>mise: 設定ファイルを trust
install_deps->>mise: mise exec -- を呼び出す
mise->>PackageManager: パッケージマネージャーを実行
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/ci.yml (1)
18-37: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win新規ジョブの
checkoutにpersist-credentials: falseの追加を検討。zizmor (artipacked) が本ジョブの
actions/checkoutにpersist-credentials: falseが設定されていない点を指摘しています。本ジョブはテスト実行のみで以降に git 資格情報を使う操作はないため実害は小さいですが、資格情報の永続化を避けるのは低コストで有効な防御策です(同パターンは既存の他ジョブにも見られるため、まとめて対応するのも一案です)。🔒 修正案
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: submodules: true + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 18 - 37, Update the actions/checkout step in the test-deps job to set persist-credentials to false, matching the existing security pattern used by other jobs where applicable. Do not alter the job’s test execution or checkout behavior otherwise.Source: Linters/SAST tools
skills/worktree-checkout/SKILL.md (1)
55-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win空行を含む blockquote (MD028) を修正してください。
新規追加した
Note (mise)ブロックと直後のNote (subagents)ブロックの間の空行が>無しになっており、markdownlint (MD028) が blockquote 内の空行として警告しています。レンダラーによっては2つの blockquote が結合される可能性があります。📝 修正案
> **Note (mise):** When a mise config applies to the new worktree, the dependency > install runs through `mise exec --` (after `mise trust`) so mise-managed > toolchains are on `PATH`. This trusts the config as checked out on the PR > branch; use `WORKTREE_NO_MISE=1` or `--no-install` for untrusted PRs. - +> > **Note (subagents):** In URL mode, `checkout` auto-cds into the new worktree🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/worktree-checkout/SKILL.md` around lines 55 - 60, Update the adjacent “Note (mise)” and “Note (subagents)” blockquotes in SKILL.md so their separating blank line is also prefixed with `>`, preserving them as distinct blockquote blocks and satisfying markdownlint MD028.Source: Linters/SAST tools
lib/deps.sh (1)
112-120: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
run_deps_cmdが失敗時の診断情報をほぼ握りつぶす。
2>&1 | tail -1で標準出力・標準エラーの最後の1行しか残らないため、npm install failed等のlog_warnと合わせても、実際に何が失敗したのか(例:npm ERR! code ETARGETのような本質的なエラー行)がログに残らず、利用者はトラブルシュートが困難になります。特にdotnet restoreやcomposer installは失敗理由が末尾より前の行に出力されることが多く、影響が大きいです。成功時は簡潔な1行ログのままでもよいですが、失敗時だけは全出力を表示する(もしくはログファイルに保存してパスを案内する)よう改善を検討してください。
♻️ 失敗時に全出力を表示する例
run_deps_cmd() { local dir="$1" shift - (cd "$dir" && "$@" 2>&1 | tail -1) + local out status + out="$(cd "$dir" && "$@" 2>&1)" + status=$? + if [ "$status" -ne 0 ]; then + echo "$out" + else + echo "$out" | tail -1 + fi + return "$status" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/deps.sh` around lines 112 - 120, Update run_deps_cmd so successful commands retain the concise final-line output, but failed commands expose the complete captured stdout and stderr for troubleshooting. Preserve the package manager’s exit status under pipefail and ensure the failure output is emitted before returning the original nonzero status.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 18-37: Update the actions/checkout step in the test-deps job to
set persist-credentials to false, matching the existing security pattern used by
other jobs where applicable. Do not alter the job’s test execution or checkout
behavior otherwise.
In `@lib/deps.sh`:
- Around line 112-120: Update run_deps_cmd so successful commands retain the
concise final-line output, but failed commands expose the complete captured
stdout and stderr for troubleshooting. Preserve the package manager’s exit
status under pipefail and ensure the failure output is emitted before returning
the original nonzero status.
In `@skills/worktree-checkout/SKILL.md`:
- Around line 55-60: Update the adjacent “Note (mise)” and “Note (subagents)”
blockquotes in SKILL.md so their separating blank line is also prefixed with
`>`, preserving them as distinct blockquote blocks and satisfying markdownlint
MD028.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5216d732-17eb-4b41-9dad-af467cd5e2eb
📒 Files selected for processing (9)
.github/workflows/ci.ymlCLAUDE.mdREADME.mdlib/cmd_checkout.shlib/cmd_create.shlib/deps.shskills/worktree-checkout/SKILL.mdskills/worktree-create/SKILL.mdtest/deps.bats
- set persist-credentials: false on every actions/checkout step. No job pushes or authenticates over git: the test suites clone public repos themselves and `gh` reads GH_TOKEN from the environment, so the checkout credentials are never needed after checkout - keep the two adjacent notes in worktree-checkout/SKILL.md as separate blockquotes (markdownlint MD028) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Dependency installation failed with
command not foundin projects whose toolchain is pinned by mise. A worktree is created by a separate process, so the shell hook that puts mise-managed tools onPATHnever fires there:Package managers now run through
mise exec --whenever a mise config applies to the worktree, and directly otherwise, so non-mise projects are unaffected.Changes
install_deps <dir> [config-root]resolves a command prefix once and applies it to every package manager (run_deps_cmdalso de-duplicates thecd+tail -1boilerplate).find_mise_configs <dir> [boundary]walks from the install directory up toboundary(the task directory) and stops there. In a multi-repo layout the config often sits in the task directory (symlinked from the project root) while the install runs in a sub-repo; the boundary keeps unrelated configs further up the tree out of scope.cmd_create.sh/cmd_checkout.shpass$task_dir.trust_mise_configspasses each*.tomlconfig tomise trustbefore running. A worktree is a brand new absolute path, so a config trusted in the project root is untrusted there, andmise execaborts (rather than prompting) in this non-interactive context whenever the config can execute code ([env], templates, tool options)..tool-versionshas no executable content and is not trusted.WORKTREE_NO_MISE=1opts out of mise integration entirely.test/deps.batswas never executed by any workflow; it now runs on ubuntu + macOS.SKILL.mdfiles.Security note
On
worktree checkout <PR URL>this trusts the mise config as checked out on the contributor's branch. Runningnpm install/composer installfrom a PR branch is already arbitrary code execution, so the marginal risk is small, butWORKTREE_NO_MISE=1or--no-installis available for untrusted PRs. This is documented in README and CLAUDE.md.Test plan
./test/bats/bin/bats test/deps.bats— 25/25 pass (12 new cases: boundary walk, normalisation of a trailing.component, non-ancestor boundary ignored,mise execrouting,mise trustinvocation,.tool-versionsnot trusted, direct execution without a config,WORKTREE_NO_MISE=1)./test/bats/bin/bats test/single_repo.bats— 84/84 pass (no regression)./test/bats/bin/bats test/multi_repo.bats— 30/30 pass (no regression)misebinary: a scratch repo with an[env]-carryingmise.toml(untrusted at the new path) plus a stubdotnetonPATH.worktree createreportedUsing mise (...)and the stub printed the env var injected by mise, proving both the trust step and themise execwrapping. Scratch trust entries were removed withmise trust --untrustafterwards.worktree checkout https://github.com/EcAuth/EcAuth/pull/458now restores the solution instead of failing.🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
mise exec経由でパッケージマネージャーを実行します。WORKTREE_NO_MISE=1により、mise連携を無効化できます。ドキュメント
テスト