Skip to content

fix(hooks): revert per-tool hook execution#7509

Merged
jdx merged 4 commits into
jdx:mainfrom
just-be-dev:revert-7311-hooks-env-vars
Dec 30, 2025
Merged

fix(hooks): revert per-tool hook execution#7509
jdx merged 4 commits into
jdx:mainfrom
just-be-dev:revert-7311-hooks-env-vars

Conversation

@just-be-dev
Copy link
Copy Markdown
Contributor

PR #7311 aimed to add MISE_TOOL_NAME and MISE_TOOL_VERSION environment variables to preinstall/postinstall hooks (fixing #7255). To support this feature, it changed the hooks to execute once per tool installation rather than once after all tools were installed.

Unfortunately, this change broke existing workflows in the wild (see #7489). The problem is that postinstall hooks now run before the newly installed tools are available in the PATH. Users commonly use postinstall hooks to run commands like pnpm install or bundle install, which depend on tools that were just installed by mise. These commands now fail with "command not found" errors because the hooks fire too early in the installation process.

Additionally, having hooks execute multiple times (once per tool) rather than once at the end of the installation session is unexpected and causes commands to run redundantly.

This PR reverts commit a319a87 to restore the previous behavior where hooks run once after all installations complete and tools are available in PATH.

@just-be-dev just-be-dev changed the title revert(hooks): remove MISE_TOOL_NAME and MISE_TOOL_VERSION from preinstall/postinstall hooks fix(hooks): revert per-tool hook execution (restores session-wide behavior) Dec 29, 2025
@just-be-dev just-be-dev changed the title fix(hooks): revert per-tool hook execution (restores session-wide behavior) fix(hooks): revert per-tool hook execution Dec 29, 2025
Complete the revert of PR jdx#7311 by removing per-tool hook execution
from toolset_install.rs (which was refactored from mod.rs in PR jdx#7371).

Removes:
- HookToolContext import
- Per-tool preinstall hook calls
- Per-tool postinstall hook calls

This restores the original behavior where hooks run once per session
rather than once per tool.
@just-be-dev just-be-dev marked this pull request as draft December 29, 2025 23:21
@just-be-dev

This comment was marked as resolved.

After PR jdx#7371 refactored code from mod.rs to toolset_install.rs,
the revert inadvertently added back duplicate functions that should
only exist in toolset_install.rs.

Removes from mod.rs:
- init_request_options()
- install_all_versions()
- install_some_versions()

Also cleans up unused imports in both files.
PR jdx#7311 removed the session-wide preinstall/postinstall hooks and replaced
them with per-tool hooks. This commit completes the revert by restoring the
original session-wide hooks that run once per installation session.

This was missed in the initial revert because PR jdx#7371 moved the code from
mod.rs to toolset_install.rs after PR jdx#7311, so the git revert didn't
automatically restore the removed hooks.
@just-be-dev just-be-dev force-pushed the revert-7311-hooks-env-vars branch from 0160bb7 to 69d878e Compare December 29, 2025 23:32
@just-be-dev just-be-dev marked this pull request as ready for review December 29, 2025 23:33
@jdx jdx merged commit d26c7a6 into jdx:main Dec 30, 2025
33 checks passed
jdx added a commit that referenced this pull request Dec 30, 2025
## Summary

Adds environment variables to postinstall hooks to address the use cases
from #7255 and #7489 without changing hook timing (avoiding the
regression that led to the revert in #7509).

### Problem

- **#7255**: Users wanted `MISE_TOOL_NAME` and `MISE_TOOL_VERSION` in
hooks for conditional logic
- **#7311**: Fixed this by running hooks per-tool, but broke existing
workflows
- **#7489**: Reported that hooks now ran BEFORE tools were in PATH,
breaking `pnpm install`, `bundle install`, etc.
- **#7509**: Reverted #7311 entirely

### Solution: Dual Enhancement

This PR provides both capabilities without changing when hooks run:

**Tool-level postinstall** (runs per-tool immediately after install):
```toml
[tools]
node = { version = "20", postinstall = "echo $MISE_TOOL_NAME@$MISE_TOOL_VERSION" }
```
- `MISE_TOOL_NAME`: short name of the tool (e.g., "node")
- `MISE_TOOL_VERSION`: version installed (e.g., "20.10.0")
- `MISE_TOOL_INSTALL_PATH`: path where tool was installed

**Global postinstall hook** (runs once after ALL tools installed):
```toml
[hooks]
postinstall = "echo $MISE_INSTALLED_TOOLS && pnpm install"
```
- `MISE_INSTALLED_TOOLS`: JSON array of installed tools, e.g.,
`[{"name":"node","version":"20.10.0"}]`
- All tools are in PATH at this point

### Why This Works

1. **No timing changes**: Hooks run at the same times as before the
revert
2. **Per-tool context**: Available via tool-level postinstall option
3. **Full PATH access**: Global hook still runs after all tools
installed
4. **Both use cases supported**: Users choose which mechanism fits their
needs

## Test plan

- [x] Added `e2e/config/test_hooks_tool_env` - tests tool-level env vars
- [x] Added `e2e/config/test_hooks_installed_tools` - tests global hook
JSON
- [x] Existing hook tests pass (`test_hooks`,
`test_hooks_postinstall_env`)
- [x] `mise run lint-fix` passes

Closes #7255
Closes #7489

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Enables postinstall hooks to receive precise install context without
changing hook timing.
> 
> - Tool-level `postinstall` now runs with `MISE_TOOL_NAME`,
`MISE_TOOL_VERSION`, and `MISE_TOOL_INSTALL_PATH` (set in
`src/backend/mod.rs`)
> - Global `postinstall` receives `MISE_INSTALLED_TOOLS` (JSON array)
via new hook context plumbing in `src/hooks.rs` and
`src/toolset/toolset_install.rs`
> - Updates `docs/hooks.md` with examples and env var descriptions
> - Adds e2e tests: `e2e/config/test_hooks_tool_env` and
`e2e/config/test_hooks_installed_tools`
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
650d8a0. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
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