fix(shell): install nushell wrapper to vendor-autoload dir (#2878)#2992
Merged
Conversation
The nushell integration wrote `wt.nu` to `$nu.default-config-dir/vendor/autoload`, a path Nushell never autoloads. It only worked on macOS/Windows by coincidence (there `$nu.default-config-dir == $nu.data-dir`); on Linux the wrapper was written but never loaded, so `wt` was never wrapped. Resolve the install target from `$nu.vendor-autoload-dirs | last` (the user-writable vendor-autoload dir under `$nu.data-dir`), with an XDG_DATA_HOME / `dirs::data_dir()` fallback that mirrors nushell's own `nu_path::data_dir` when `nu` isn't on PATH. The legacy `<config-dir>/vendor/autoload` locations are kept as detection candidates so install (and uninstall) clean up files stranded by older versions. - paths: query `$nu.vendor-autoload-dirs`/`$nu.default-config-dir`, build canonical + fallback + legacy candidates - install: write to the canonical vendor-autoload dir and remove worktrunk-managed wrappers stranded at legacy locations - config_line / docs: use `$nu.vendor-autoload-dirs | last | path join` - add a `WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR` override so tests are deterministic across platforms, plus a real-`nu` guard asserting the install target is a member of `$nu.vendor-autoload-dirs` Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers the data-safety branch where a legacy <config-dir>/vendor/autoload wt.nu lacking the worktrunk header is left untouched on install. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
worktrunk-bot
commented
Jun 6, 2026
worktrunk-bot
left a comment
worktrunk-bot
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed as the PR author (self-review) — submitting as a comment, not an approval.
Data-safety surface — flagging for human review. This PR adds a new file-deletion path: cleanup_stranded_nushell in src/commands/configure_shell.rs calls fs::remove_file to delete wt.nu from <config-dir>/vendor/autoload during wt config shell install. Per the repo's deletion-surface policy, any change that introduces std::fs::remove_file is held for a maintainer rather than auto-approved.
The guard rails look right to me, for the record:
- It only removes files whose contents carry the
# worktrunk shell integration for nushellheader (is_worktrunk_managed_nushell), so a user's ownwt.nuis left alone —test_nushell_install_keeps_unmanaged_legacy_filecovers exactly this. - It never targets the canonical write path (
if &path == canonical { continue }). - A failed removal warns rather than aborting the install.
Calling it out so @max-sixty can eyeball the new deletion path before merge.
Overlaps with #2992 sibling work. PR #2864 ("validate command names and tighten install/uninstall detection") touches the same files — src/commands/configure_shell.rs, src/shell/mod.rs, src/shell/detection.rs, src/shell/paths.rs, tests/integration_tests/configure_shell.rs. Different concern, not a duplicate, but whichever lands second will need a rebase through these conflicts.
Everything else looks sound: the $nu.vendor-autoload-dirs | last resolution matches Nushell's autoload contract, the nu-absent fallback mirrors nu_path::data_dir, docs/help/snapshots are updated consistently, and the real-nu guard (test_nushell_install_target_is_a_vendor_autoload_dir) directly asserts the bug is fixed.
The new nushell install/uninstall tests failed on macOS and Windows
because the vendor-autoload override path didn't match the HOME that
`set_temp_home_env` exports (via `dunce::canonicalize`):
- macOS: tests built the override from the raw `temp_home.path()` under
`/var` (a symlink to `/private/var`), so the exported, canonicalized
HOME `/private/var/...` failed to strip-prefix it and the path printed
in full instead of `~/...` (config_show_nushell_outdated_wrapper,
uninstall_shell, uninstall_shell_multiple).
- Windows: tests used `std::fs::canonicalize`, which returns a `\\?\`
verbatim path; the forward slashes in the subsequent
`.join(".local/share/...")` are then literal filename characters, not
separators, so the wrapper write and `.exists()` check missed
(the six configure_shell nushell tests).
Add a `canonical_temp_home` helper that uses `dunce::canonicalize` — it
resolves the macOS symlink (matching the exported HOME) while returning
an ordinary, non-verbatim path on Windows — and route every nushell
override path through it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
|
OK, let's try this. It looks reasonable but admittedly I have not looked through each line or tested locally... I would like to be more conscientious about these; it's harder on @nnutter, great if you can test on you end, thank you... |
Merged
max-sixty
added a commit
that referenced
this pull request
Jun 9, 2026
v0.57.0. `cargo semver-checks` reports breaking library API changes: the squash refactor (#2983) renamed `Repository::commit_subjects` to `commit_message_details`, and the new `GitError::UnbornDefaultBranch` variant (#2990) shifts the discriminants of the exhaustive `GitError` enum. Neither is intentional public API, but together they force a minor bump pre-1.0. ## CHANGELOG ### Improved - **`wt step diff --branch`**: `wt step diff` gained a `-b`/`--branch` flag, mirroring `wt step commit`, so the diff can target another worktree's branch without leaving the current one. The branch must have a checked-out worktree. ([#2995](#2995)) - **Squash templates can use commit bodies**: The squash commit-message template gains an experimental `{{ commit_details }}` variable — a list of `{ subject, body }` objects for the commits being squashed — alongside the existing `{{ commits }}` (now documented as the commit subjects). Templates can incorporate full commit bodies, not just subject lines. ([#2983](#2983), thanks @florianilch) - **Recommended Claude Code commit command drops the `CLAUDECODE=` prefix**: Claude Code removed the nested-session check that rejected `claude -p` launched from inside another session, so the workaround is gone. The recommended `[commit.generation]` command shown by `wt config create` no longer carries a leading `CLAUDECODE=`, and `wt` no longer strips `CLAUDECODE` from the environment before running commit-generation commands. ([#2979](#2979)) ### Fixed - **Nushell wrapper installs where Nushell actually autoloads it**: `wt config shell install nu` wrote `wt.nu` to `$nu.default-config-dir/vendor/autoload`, which Nushell never autoloads — on Linux the wrapper was written but silently never loaded, so `wt` was never wrapped (it happened to work on macOS/Windows by coincidence of path layout). It now installs to `$nu.vendor-autoload-dirs | last`, and install/uninstall clean up any worktrunk wrapper stranded at the old location. ([#2992](#2992), thanks @nnutter for reporting) - **Claude Code hooks work for Fish shell users**: The plugin's hook commands used `${CLAUDE_PLUGIN_ROOT}` brace syntax, which Fish doesn't expand; they now use `$CLAUDE_PLUGIN_ROOT`, so the activity and worktree-lifecycle hooks fire correctly under Fish. ([#2962](#2962), thanks @amw) - **Pager no longer wedges the terminal on Ctrl-C (Windows)**: Interrupting the `--help` pager (`less`) with Ctrl-C on Windows could leave the terminal in a broken state; `less` now quits cleanly on interrupt. ([#2969](#2969), thanks @ofek for reporting) - **Clearer error when the default branch has no commits**: In a freshly initialized repo whose default branch is unborn, `merge`/`rebase`/`squash`/`push` (and the diff report) failed with `Default branch main does not exist locally` plus a misleading hint to reset the cached value. They now report that the branch has no commits yet, without the wrong cache-reset suggestion. ([#2990](#2990)) - **`diagnostic.md` uploads as a gist again**: The `-vv` diagnostic report inlined raw NUL bytes from NUL-separated git output, so `gh gist create` rejected it as a binary file. Control bytes in the subprocess preview are now escaped. ([#2991](#2991)) - **`wt list` tolerates a missing index file**: A repo with no `<gitdir>/index` (nothing ever staged) made the temp-index probe fail; a missing index is now treated as an empty one, matching git's own behavior. ([#2884](#2884)) - **Inline code renders in `--help` section headings**: Terminal `--help` showed literal backticks in headings authored with inline code (e.g. the `wt config state logs` heading `Command log (`commands.jsonl`)`). Headings now reduce inline code to plain text under the heading's uniform style. The `--stage`/`--dry-run` subsection headings in `wt step commit`/`squash` were also renamed to sentence case ("Staging", "Dry run"). ([#3003](#3003)) ### Documentation - **`wt switch` docs give forge PR/MR URLs equal billing with `pr:`/`mr:`**: The switch docs now present the full forge-URL form alongside the `pr:N` shortcut. ([#2970](#2970)) - **New FAQ entry on moving uncommitted changes to a new worktree**. ([#3002](#3002)) ### Internal - **Bare-repo prompt opt-out stored as a hint**: `worktrunk.skip-bare-repo-prompt` moved under the `worktrunk.hints.` namespace, so it now lists under `wt config state hints` and clears with `wt config state clear` (previously a top-level key that escaped both). Clean cutover: users who already opted out are re-prompted once on their next `wt switch --create` in a dotted-name bare repo. ([#3001](#3001)) > _This was written by Claude Code on behalf of max_
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2878.
Implements the approach agreed in the issue: query
$nu.vendor-autoload-dirsdirectly and install to its last (user-writable) entry, with cross-platform tests and stranded-file cleanup on install/uninstall.The bug
The nushell integration wrote
wt.nuto$nu.default-config-dir/vendor/autoload, which Nushell never autoloads. It happened to work on macOS/Windows because there$nu.default-config-dir == $nu.data-dir, so the wrong subpath resolved to the real vendor dir; on Linux the wrapper was written but silently never loaded — sowtwas never wrapped.Verified against
nu0.113.1:$nu.vendor-autoload-dirs | lastis<data-dir>/vendor/autoload, and$nu.default-config-dir/vendor/autoloadis in neither$nu.vendor-autoload-dirsnor$nu.user-autoload-dirs.The fix
$nu.vendor-autoload-dirs | last(one memoisednuspawn that also reads$nu.default-config-dirfor legacy cleanup).nuisn't on PATH mirrors nushell's ownnu_path::data_dir:XDG_DATA_HOME(when absolute) wins on every platform, otherwisedirs::data_dir()— matching nushell on Linux/macOS/Windows.paths.first()), so a wrapper stranded at a legacy path can't become the write target. (No-op for fish, which has a single canonical path.)<config-dir>/vendor/autoloadlocations; uninstall already iterates every candidate, which now includes those legacy paths. Legacy files are only removed when they carry the# worktrunk shell integration for nushellheader, so a user's ownwt.nuis never touched.config_line, thewt config shell initmanual-setup help, and the FAQ/shell-integration docs now use$nu.vendor-autoload-dirs | last | path join wt.nu.Tests
WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIRoverride pins the install dir so the install/uninstall/cleanup tests are deterministic on Linux, macOS, and Windows and don't depend onnubeing installed.test_nushell_install_target_is_a_vendor_autoload_dir(gated onshell-integration-tests, where CI hasnu) runs the realnuflow and asserts the wrapper worktrunk wrote is a member of$nu.vendor-autoload-dirs. This fails againstmain(the old config-dir path is in neither autoload list) and passes with the fix.test_nushell_install_cleans_stranded_legacyand a reworkedtest_uninstall_nushell_cleans_all_candidate_locationscover the migration cleanup.cargo test --lib+cargo test --test integration, and the nushell subset with realnuunder--features shell-integration-tests).Not in scope
"Not best practice" cleanup flagged in the issue (the overlap between the candidate-building helpers, and the dubious Windows branch) — the helpers here are restructured around the autoload model, but a broader tidy can be a follow-up.
🤖 Generated with Claude Code