Skip to content

fix(vfox): run pre_uninstall hook#9662

Merged
jdx merged 4 commits into
jdx:mainfrom
risu729:fix/vfox-pre-uninstall-hook
May 10, 2026
Merged

fix(vfox): run pre_uninstall hook#9662
jdx merged 4 commits into
jdx:mainfrom
risu729:fix/vfox-pre-uninstall-hook

Conversation

@risu729

@risu729 risu729 commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add the vfox PreUninstall hook binding and context
  • run vfox tool plugin pre_uninstall hooks before removing install directories
  • preserve dry-run behavior and skip the hook when the plugin itself is unavailable
  • add crate and e2e regression coverage

Source

Tests

  • cargo test -p vfox
  • mise run test:e2e e2e/backend/test_vfox_pre_uninstall

@greptile-apps

greptile-apps Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds support for the vfox PreUninstall hook, which fires before the install directory is removed during mise uninstall. Dry-run mode and the case where the plugin is unavailable are both handled correctly.

  • Adds crates/vfox/src/hooks/pre_uninstall.rs with PreUninstallContext and the Plugin::pre_uninstall executor, paralleling the existing post_install hook.
  • Overrides uninstall_version_impl in src/backend/vfox.rs to call the hook only when the plugin is installed and not a backend plugin, with logging wired through the existing log_rx pattern.
  • Adds unit and e2e tests that verify the marker file is written on a real uninstall and not written during a dry-run.

Confidence Score: 5/5

The change is self-contained: the new hook runs only when explicitly declared by the plugin, dry-run is preserved via the existing base-trait guard, and all new code follows established patterns in the vfox backend.

The hook execution is gated by a metadata check so plugins without pre_uninstall are unaffected. The dry-run guard lives in the base uninstall_version method, which is not modified here, so that invariant cannot regress. Error propagation from the hook correctly aborts the uninstall before directory removal. The unit test and e2e test together cover the happy path, the dry-run path, and the marker content.

No files require special attention.

Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/vfox-pre-un..." | Re-trigger Greptile

Comment thread src/backend/vfox.rs Outdated

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a pre_uninstall hook for vfox plugins, allowing them to perform cleanup tasks before an SDK version is removed. The changes include the necessary Rust infrastructure to execute the hook, an update to the uninstall method to make it asynchronous, and the addition of unit and end-to-end tests. Feedback was provided regarding the uninstall method's transition to an asynchronous function, which represents a breaking change for the vfox crate API.

Comment thread crates/vfox/src/vfox.rs Outdated
@risu729

This comment was marked as outdated.

@risu729 risu729 marked this pull request as ready for review May 8, 2026 13:37
@jdx jdx merged commit 4e43654 into jdx:main May 10, 2026
35 checks passed
@risu729 risu729 deleted the fix/vfox-pre-uninstall-hook branch May 10, 2026 13:30
mise-en-dev added a commit that referenced this pull request May 11, 2026
### 🚀 Features

- **(cli)** add minimum release age flag to lock and ls-remote by
@risu729 in [#9269](#9269)
- **(config)** add run field for hooks by @risu729 in
[#9718](#9718)
- **(github)** add native oauth token source by @jdx in
[#9654](#9654)
- **(oci)** scope build to project config by default by @jdx in
[#9766](#9766)
- add support for prefixed latest version queries in outdated checks by
@roele in [#9767](#9767)

### 🐛 Bug Fixes

- **(activate)** guard bash chpwd hook under nounset by @risu729 in
[#9716](#9716)
- **(backend)** date-check latest stable fast path by @risu729 in
[#9650](#9650)
- **(config)** parse core tool options consistently by @risu729 in
[#9742](#9742)
- **(exec)** propagate __MISE_DIFF so nested mise recovers pristine PATH
by @jdx in [#9765](#9765)
- **(forgejo)** include prereleases when opted in by @risu729 in
[#9717](#9717)
- **(github)** avoid caching empty release assets by @risu729 in
[#9616](#9616)
- **(java)** resolve lockfile URLs from metadata by @risu729 in
[#9719](#9719)
- **(lock)** cache unavailable github attestations by @risu729 in
[#9741](#9741)
- **(pipx)** preserve options when reinstalling tools by @risu729 in
[#9663](#9663)
- **(python)** skip redundant lockfile provenance verification by
@risu729 in [#9739](#9739)
- **(vfox)** run pre_uninstall hook by @risu729 in
[#9662](#9662)

### 🚜 Refactor

- **(schema)** extract tool options definition by @risu729 in
[#9649](#9649)

### ⚡ Performance

- **(aqua)** bake rkyv aqua package blobs by @risu729 in
[#9535](#9535)

### 📦️ Dependency Updates

- lock file maintenance by @renovate[bot] in
[#9773](#9773)

### 📦 Registry

- add vector
([github:vectordotdev/vector](https://github.com/vectordotdev/vector))
by @kquinsland in [#9761](#9761)
- add oc and openshift-install (http backend) by @konono in
[#9669](#9669)

### New Contributors

- @konono made their first contribution in
[#9669](#9669)
- @kquinsland made their first contribution in
[#9761](#9761)
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