Skip to content

fix(global): strip Windows \\?\\ verbatim prefix from canonicalized install dir#243

Merged
jdx merged 2 commits intomainfrom
fix/windows-verbatim-canonicalize
Apr 23, 2026
Merged

fix(global): strip Windows \\?\\ verbatim prefix from canonicalized install dir#243
jdx merged 2 commits intomainfrom
fix/windows-verbatim-canonicalize

Conversation

@jdx
Copy link
Copy Markdown
Contributor

@jdx jdx commented Apr 23, 2026

Summary

run_global calls std::fs::canonicalize on the freshly-created install directory and on layout.pkg_dir. On Windows those calls return the extended-length form \\?\C:\…. Every downstream step that concatenates this dir into another path then carries the verbatim prefix forward, and the bin-shim writer in particular ends up baking it into the relative target string it interpolates into %~dp0\{rel}.

The resulting .cmd points at <bin_dir>\\\\?\\C:\…\prettier.cjs, which cmd.exe collapses to <bin_dir>\?\C:\…\prettier.cjs, which Node then refuses with:

Cannot find module 'C:\…\bin\?\C:\…\node_modules\prettier\bin\prettier.cjs'

There is already an inline fix for the same class of bug for the per-dep bin dir at aube::commands::install::link_bin_via_dep. This PR generalizes it into a reusable dirs::canonicalize helper (matches dunce::canonicalize without adding the dep) and routes the three run_global call sites through it so the install dir, the layout pkg dir, and the symmetric post-install equality checks all see the same plain drive-letter form.

Approach

  • New aube::dirs::canonicalize (in crates/aube/src/dirs.rs) — runs std::fs::canonicalize, then strips the \\?\ prefix on Windows when the result wasn't a real \\?\UNC\… share path. No-op on Unix.
  • Three call sites in crates/aube/src/commands/add.rs::run_global switch from std::fs::canonicalizecrate::dirs::canonicalize. Two of them are equality checks against install_dir; if install_dir is now stripped, the other side has to be stripped the same way or the comparisons silently regress.

Reproduced via

mise's windows-e2e tranche, after switching mise's npm-publish to aube and triggering mise install npm:[email protected]. The CI run that surfaced it: https://github.com/jdx/mise/actions/runs/24852804095.

Test plan

  • cargo test --bin aube281 pass (1 new: canonicalize_round_trips_an_existing_path; plus a #[cfg(windows)] guard test canonicalize_strips_verbatim_drive_prefix)
  • cargo fmt --check
  • cargo clippy --all-targets
  • Manual repro on Windows is gated on landing this and re-running mise's windows-e2e — confident the fix is correct from the existing precedent at install/mod.rs:4814

🤖 Generated with Claude Code


Note

Medium Risk
Touches global install/bin-linking path canonicalization and cleanup logic; mainly Windows-specific but could affect how paths compare and which directories get cleaned up.

Overview
Fixes Windows global installs by routing canonicalization through a new crate::dirs::canonicalize helper that strips the \\?\ verbatim-drive prefix (while preserving real \\?\UNC\… shares).

add -g and global package scanning/removal now use this helper so path comparisons (==/starts_with) and bin-shim target construction don’t break on Windows, preventing broken .cmd shims and leaked/stale global install dirs/hash pointers. Bin-linking’s Windows mkdir workaround is simplified to rely on the same helper, and new unit tests cover the canonicalize behavior.

Reviewed by Cursor Bugbot for commit 6018f5e. Bugbot is set up for automated code reviews on this repo. Configure here.

…stall dir

`run_global` calls `std::fs::canonicalize` on the freshly-created install
directory and on `layout.pkg_dir`. On Windows those calls return the
extended-length form `\\?\C:\…`. Every downstream step that concatenates
this dir into another path then carries the verbatim prefix forward, and
the bin-shim writer in particular ends up baking it into the relative
target string it interpolates into `%~dp0\{rel}`. The resulting `.cmd`
points at `<bin_dir>\\\\?\\C:\…\prettier.cjs`, which `cmd.exe` collapses
to `<bin_dir>\?\C:\…\prettier.cjs`, which Node then refuses with:

    Cannot find module 'C:\…\bin\?\C:\…\node_modules\prettier\bin\prettier.cjs'

There is already an inline fix for the same class of bug for the per-dep
bin dir at `aube::commands::install::link_bin_via_dep`. This PR generalizes
it into a reusable `dirs::canonicalize` helper (matches `dunce::canonicalize`
without adding the dep) and routes the three `run_global` call sites
through it so the install dir, the layout pkg dir, and the symmetric
post-install equality checks all see the same plain drive-letter form.

Reproduced via mise's `windows-e2e` job:
https://github.com/jdx/mise/actions/runs/24852804095 (the
`mise install npm:[email protected] -- prettier --version` step).

Validation:
- `cargo test --bin aube` — 281 pass (1 new, `canonicalize_round_trips_an_existing_path`;
  also a `#[cfg(windows)]` `canonicalize_strips_verbatim_drive_prefix`)
- `cargo fmt --check`
- `cargo clippy --all-targets`

Signed-off-by: jdx <[email protected]>
jdx added a commit to jdx/mise that referenced this pull request Apr 23, 2026
…oml"

Re-add aube to mise.toml and route the workflow back through `mise install
aube` + `mise x aube --`. The Windows-e2e regression that motivated the
previous commit was actually a bug in aube's global-install bin shim
on Windows (canonicalize returning the `\\?\` verbatim prefix); fixed
upstream in endevco/aube#243. This PR depends on
that landing in an aube release.

This reverts commit 1c3bca3.
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 23, 2026

Greptile Summary

Introduces crate::dirs::canonicalize — a thin wrapper over std::fs::canonicalize that strips the Windows \\?\ verbatim-drive prefix (while preserving real \\?\UNC\ share paths) — and routes every path-canonicalization call site in run_global, scan_packages, remove_package, and bin_linking through it for consistency.

Both previously flagged findings are resolved: scan_packages now returns non-verbatim paths (fixing the prior-cleanup equality mismatch), and the inline verbatim-strip in bin_linking.rs is replaced with the helper (gaining the UNC guard it previously lacked). The implementation is well-tested and the UNC-preservation logic is correct.

Confidence Score: 5/5

Safe to merge — all prior P1/P2 findings are addressed and no new issues were found.

Both previously raised concerns (stale hash pointers from verbatim-prefixed scan_packages paths, and the inline strip in bin_linking.rs lacking the UNC guard) are fully resolved. The new helper is correct, well-documented, and covered by unit tests on both platforms. No remaining P0/P1 findings.

No files require special attention.

Important Files Changed

Filename Overview
crates/aube/src/dirs.rs New canonicalize helper: calls std::fs::canonicalize, then on Windows strips \\?\ prefix unless it's a real \\?\UNC\ share path. Well-documented, cross-platform (no-op on Unix), includes unit tests for both the round-trip case and the Windows-only prefix-stripping assertion.
crates/aube/src/commands/add.rs Routes all four run_global / run_global_inner canonicalization call sites through crate::dirs::canonicalize so install_dir, layout.pkg_dir, the error-cleanup equality check, and the prior-detection check all use consistent non-verbatim paths on Windows.
crates/aube/src/commands/global.rs Both scan_packages and remove_package now use crate::dirs::canonicalize; addresses the prior P1 (stale-hash-pointer accumulation because scan_packages was still returning verbatim \\?\ paths) and ensures starts_with in remove_package stays consistent.
crates/aube/src/commands/install/bin_linking.rs Replaces the inline verbatim-strip (missing UNC guard) with crate::dirs::canonicalize, addressing the prior P2 and eliminating the code duplication in one step.

Reviews (2): Last reviewed commit: "fix(global): route scan_packages and bin..." | Re-trigger Greptile

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 27347e6. Configure here.

Comment thread crates/aube/src/commands/add.rs
… dirs::canonicalize

Two follow-on fixes to the same Windows verbatim-prefix class of bug as the
parent commit:

- `global::scan_packages` (and via it, `find_package`) populated
  `GlobalPackageInfo.install_dir` from `std::fs::canonicalize`, so the
  field still carried `\\?\C:\…`. The new `run_global_inner` prior-cleanup
  code compares those paths against `install_dir` (now stripped via
  `dirs::canonicalize`) at `add.rs:1064-1077`. Without this fix the
  comparisons never match on Windows and stale install dirs leak every
  reinstall of the same alias set. `remove_package` had the same shape —
  `pkg_canon` was verbatim, but `info.install_dir` is now stripped, so the
  `starts_with` guard would fail and the install dir would never be
  removed.

- `commands::install::bin_linking` carried an inline `s.strip_prefix(r"\\?\")`
  that the parent commit's helper was extracted from. The inline version
  unconditionally strips the prefix, including from real `\\?\UNC\server\share`
  paths that have no non-verbatim equivalent. Replacing it with
  `dirs::canonicalize` (which preserves UNC share paths) keeps the
  behavior identical for the only path it ever sees in practice today
  (drive-letter junctions) and removes the only divergent copy of this
  logic.

Greptile flagged both during review of #243 (P1 + P2).

Validation:
- `cargo test --bin aube` — 281 pass
- `cargo fmt --check`, `cargo clippy --all-targets`

Signed-off-by: jdx <[email protected]>
@jdx jdx merged commit 26ca405 into main Apr 23, 2026
16 checks passed
@jdx jdx deleted the fix/windows-verbatim-canonicalize branch April 23, 2026 20:43
jdx added a commit to jdx/mise that referenced this pull request Apr 24, 2026
…oml"

Re-add aube to mise.toml and route the workflow back through `mise install
aube` + `mise x aube --`. The Windows-e2e regression that motivated the
previous commit was actually a bug in aube's global-install bin shim
on Windows (canonicalize returning the `\\?\` verbatim prefix); fixed
upstream in endevco/aube#243. This PR depends on
that landing in an aube release.

This reverts commit 1c3bca3.
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.

1 participant