Skip to content

fix(hooks): skip tool installation in preinstall tasks - #11927

Merged
jdx merged 1 commit into
jdx:mainfrom
Marukome0743:pr11927
Aug 13, 2026
Merged

jdx merged 1 commit into
jdx:mainfrom
Marukome0743:pr11927

Conversation

@Marukome0743

@Marukome0743 Marukome0743 commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • run task-backed preinstall hooks with mise run --skip-tools so they complete before project or task tools are installed
  • share task-hook argument construction between execution and dry-run previews
  • document the preinstall tool-availability boundary and add ordering, interactive I/O, dry-run, and postinstall regression coverage

Root cause

The outer install pipeline invoked the preinstall hook at the correct point, but a { task = "..." } hook launched a nested mise run. That nested run performed its normal automatic tool installation before executing the task, so the nominal preinstall task observed the tools as already installed.

Only task-backed preinstall hooks now add --skip-tools. Enter, leave, cd, and postinstall task hooks retain normal task tool handling.

Verification

  • mise exec -- cargo test --all-features hooks::tests (12 passed)
  • mise run test:e2e e2e/config/test_hooks_task_ref
  • related hook E2E suite exercised through e2e/config/test_hooks
  • macOS real-TTY credential prompt: input and multiline output preserved; hook completed before the outer tool install
  • mise exec -- cargo fmt --all -- --check
  • mise exec -- cargo check --all-features
  • mise exec -- cargo clippy --workspace --all-features --all-targets -- -D warnings
  • scoped ShellCheck, shfmt, and Markdown checks passed; the aggregate local mise run lint reached an unrelated actionlint repository-discovery limitation because this checkout uses Sapling without a .git directory

Addresses #8800

AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed task-backed preinstall hooks so they no longer automatically install missing project- or task-level tools.
    • Preserved standard tool installation for other task-backed hook types.
    • Improved hook preview and execution consistency.
  • Tests

    • Added coverage for preinstall ordering, dry runs, interactive input inheritance, postinstall execution, and tool installation behavior.
  • Documentation

    • Clarified tool installation behavior for task-backed preinstall hooks.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 16db0c72-7dc1-4c1e-a60e-27cf568048e9

📥 Commits

Reviewing files that changed from the base of the PR and between 44dc62d and f98a8fa.

📒 Files selected for processing (3)
  • docs/hooks.md
  • e2e/config/test_hooks_task_ref
  • src/hooks.rs

📝 Walkthrough

Walkthrough

Task-backed preinstall hooks now skip automatic installation of missing project- and task-level tools. Shared command construction applies this behavior to execution and previews. Tests and documentation cover ordering, interaction, postinstall behavior, and tool installation.

Changes

Task hook behavior

Layer / File(s) Summary
Centralize task hook command handling
src/hooks.rs
Task hook previews and execution use shared argument construction. Preinstall hooks receive --skip-tools; other hook types retain standard arguments. Unit tests verify both paths.
Validate and document hook behavior
e2e/config/test_hooks_task_ref, docs/hooks.md
End-to-end coverage checks dry-run behavior, interactive input, hook ordering, postinstall execution, and final tool installation. Documentation describes preinstall tool behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HookDispatcher
  participant task_hook_args
  participant TaskRunner
  HookDispatcher->>task_hook_args: Build task hook arguments
  task_hook_args->>TaskRunner: Pass preinstall arguments with --skip-tools
  TaskRunner-->>HookDispatcher: Execute or preview task hook
Loading

Suggested reviewers: jdx, risu729, jambalaya56562

Poem

A rabbit hops through hooks with care,
Skips missing tools before install air.
Credentials flow, then tasks proceed,
Postinstall follows at proper speed.
Tests and docs mark every trail.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title clearly and concisely describes the main change: skipping tool installation for preinstall task hooks.

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.

❤️ Share

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

@Marukome0743
Marukome0743 marked this pull request as ready for review August 13, 2026 18:15
@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR prevents task-backed preinstall hooks from triggering nested automatic tool installation and aligns dry-run output with the actual invocation.

  • Adds --skip-tools only to task-backed preinstall hook execution.
  • Centralizes task-hook argument construction for runtime and preview paths.
  • Documents the preinstall tool-availability boundary.
  • Adds unit and end-to-end coverage for ordering, interactive input, dry runs, and postinstall behavior.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code defects identified.

The runtime and preview paths share correctly ordered arguments, preinstall alone skips automatic tool installation as intended, and the documented behavioral boundary is covered by focused unit and end-to-end tests.

Important Files Changed

Filename Overview
src/hooks.rs Centralizes task-hook arguments and correctly adds --skip-tools before the task name only for preinstall hooks, with focused unit coverage.
e2e/config/test_hooks_task_ref Extends hook task-reference coverage to verify preinstall ordering, inherited input, dry-run behavior, tool installation, and postinstall execution.
docs/hooks.md Clearly documents that task-backed preinstall hooks require commands to be available independently of project- or task-level automatic installation.

Reviews (1): Last reviewed commit: "fix(hooks): skip tool installation in pr..." | Re-trigger Greptile

@jdx
jdx merged commit 433e2e5 into jdx:main Aug 13, 2026
31 checks passed
@Marukome0743
Marukome0743 deleted the pr11927 branch August 13, 2026 18:44
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.

2 participants