Skip to content

fix: Windows CLI uninstall no longer leaves stale commands behind#1169

Merged
hatayama merged 8 commits into
v3-betafrom
codex/restart-windows-cli-uninstall-goal
May 19, 2026
Merged

fix: Windows CLI uninstall no longer leaves stale commands behind#1169
hatayama merged 8 commits into
v3-betafrom
codex/restart-windows-cli-uninstall-goal

Conversation

@hatayama
Copy link
Copy Markdown
Owner

Summary

  • Windows CLI uninstall now removes the package-owned launcher and User PATH entry instead of leaving stale commands behind.
  • Installing the native CLI from the Unity UI now cleans up legacy npm uloop-cli shims more reliably before making the native launcher first on PATH.
  • The Unity UI waits for deferred Windows uninstall cleanup so it does not show a false failure while the cleanup process is still finishing.

User Impact

  • After uninstalling the V3 CLI, a new terminal should no longer resolve uloop from the package-owned Windows install directory.
  • Users upgrading from the V2 npm CLI to the V3 native CLI get the old npm launcher removed so the new native command is the one that runs.
  • The Settings/Setup UI reports uninstall completion only after the launcher and User PATH entry are actually gone.

Changes

  • Update the Windows installer script to put the native install directory first in User PATH and remove legacy npm artifacts from both command shims and package folders.
  • Make the Unity UI uninstall path call the package-bundled native CLI when available, then wait for the launcher and Windows User PATH cleanup to finish.
  • Remove the obsolete Windows uninstall workaround that recreated the install directory after cleanup.
  • Regenerate checked-in native CLI binaries for macOS and Windows.

Verification

  • go test ./internal/uninstall ./internal/cli
  • scripts/check-go-cli.sh
  • Packages/src/Cli~/dist/windows-amd64/uloop.exe compile --project-path C:\Users\booql\oss\unity-cli-loop
  • Packages/src/Cli~/dist/windows-amd64/uloop.exe run-tests --project-path C:\Users\booql\oss\unity-cli-loop --test-mode EditMode --filter-type regex --filter-value NativeCliInstallerTests
  • Windows uninstall smoke test with a temporary install directory confirmed uloop.exe is removed and no legacy marker is created.

hatayama added 6 commits May 19, 2026 10:53
Remove the native install directory from User PATH during Windows uninstall so the command disappears from new shells. Harden the Windows installer to clean PATH-visible uloop-cli npm shims and packages before and after native install, then refresh the checked-in native binaries.
Ensure Windows uninstall removes the persistent User PATH entry before deleting the launcher, and make the Unity UI wait for both the launcher and User PATH entry to disappear. This prevents V3 uninstall from leaving a stale command directory that can confuse older V2 detection while preserving full command-not-found behavior after uninstall.
Run Windows editor uninstall through the CLI binary bundled with the current package when it is available. This lets the V3 UI recover from older installed launchers that can delete uloop.exe while leaving the persistent User PATH entry behind.
Leave an empty package-owned install directory after Windows uninstall so stale Unity Hub process PATH entries do not point at a missing directory while the launcher binary and User PATH entry are still removed.
Allow Settings uninstall to wait for deferred Windows PowerShell cleanup before reporting failure, so the UI does not show a failed uninstall after the launcher and User PATH entry are removed.
Drop the Unity 6.4/6.5 investigation workaround that recreated the Windows install directory and cleaned up an unused legacy marker. The workaround does not prevent the newer Unity Win32Exception path and it adds unnecessary uninstall behavior beyond removing the launcher and PATH entry.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

Review Change Stack

Warning

Rate limit exceeded

@hatayama has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 51 minutes and 7 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fef64c36-6422-4f74-a513-f1c7e940eeba

📥 Commits

Reviewing files that changed from the base of the PR and between 334c777 and 521f0af.

📒 Files selected for processing (2)
  • Assets/Tests/Editor/NativeCliInstallerTests.cs
  • Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs
📝 Walkthrough

Walkthrough

This PR extends Windows uninstall to remove the native CLI directory from Windows User PATH, adds completion verification that checks both file removal and PATH absence, and supplies platform-aware completion messaging and path utilities across the Go CLI and C# orchestration layers.

Changes

Windows Uninstall with PATH Cleanup

Layer / File(s) Summary
Configuration Constants
Packages/src/Editor/Domain/CliConstants.cs, Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs
Added WINDOWS_AMD64_DIST_DIR_NAME and UNINSTALL_COMPLETION_TIMEOUT_MS constants for distribution directory naming and timeout configuration.
Go CLI Uninstall Messaging
Packages/src/Cli~/internal/cli/uninstall.go, Packages/src/Cli~/internal/cli/uninstall_test.go
Added writeUninstallPathCompletion helper that prints Windows-specific User PATH removal message or non-Windows manual cleanup instruction; updated help text to reflect OS-specific PATH behavior; includes platform-specific test coverage.
Go Windows Deletion Script Expansion
Packages/src/Cli~/internal/uninstall/command.go, Packages/src/Cli~/internal/uninstall/command_test.go
Expanded PowerShell deletion script with parent-process wait, path normalization, and USER PATH cleanup that removes matching install-directory entries (case-insensitive) while preserving other entries; tests verify PATH removal occurs before file deletion and expect the richer script sequence.
C# Uninstall Command Building & Completion Logic
Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs, Assets/Tests/Editor/NativeCliInstallerTests.cs
Refactored uninstall to prefer a package-local Windows CLI when present, replaced completion polling to check both file removal and Windows User PATH absence (conditionally), removed install directory from the current process PATH after success, and added tests covering success/failure/timeouts and command selection fallback.
C# Path Detection & Manipulation Helpers
Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs
Added utilities to build a PATH without the install directory, remove install-directory entries from the current process PATH, resolve package-local CLI layout, and determine whether the user PATH check should be included in completion polling.
Shell Integration Tests
scripts/test-install-release-filter.sh
Updated PowerShell test assertions to match reworked legacy-NPM cleanup helper names and the revised uninstall invocation/branching patterns.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • hatayama/unity-cli-loop#1038: Introduces initial Windows native CLI uninstall and User PATH cleanup flow in NativeCliInstaller; this PR extends that foundation with PATH-based completion verification and removal helpers.
  • hatayama/unity-cli-loop#1154: Modifies NativeCliInstaller uninstall completion logic; related at the core uninstall flow level as both PRs alter completion waiting and PATH handling behavior.
  • hatayama/unity-cli-loop#1135: Adds the uloop uninstall command implementation in Go; this PR provides the Windows-specific message output and PowerShell deletion script enhancements that the command invokes.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: Windows CLI uninstall no longer leaves stale commands behind' directly and clearly describes the main objective of the changeset: preventing stale commands from remaining after Windows CLI uninstall.
Description check ✅ Passed The description is well-related to the changeset, providing a comprehensive summary of Windows uninstall improvements, legacy npm cleanup, and User PATH handling that aligns with the actual code changes.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/restart-windows-cli-uninstall-goal

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 and usage tips.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 12 files

Re-trigger cubic

Keep Windows installs successful when the native uloop binary already owns persisted PATH even if legacy npm cleanup cannot finish cleanly. Also let fallback uninstall launchers complete without requiring User PATH removal they do not own, and align Go uninstall PATH normalization with the Unity wait logic.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 901181c9-5cce-453c-959f-3a853782ecfe

📥 Commits

Reviewing files that changed from the base of the PR and between 68e8532 and 334c777.

⛔ Files ignored due to path filters (4)
  • Packages/src/Cli~/dist/darwin-amd64/uloop is excluded by !**/dist/** and included by none
  • Packages/src/Cli~/dist/darwin-arm64/uloop is excluded by !**/dist/** and included by none
  • Packages/src/Cli~/dist/windows-amd64/uloop.exe is excluded by !**/dist/**, !**/*.exe and included by none
  • scripts/install.ps1 is excluded by none and included by none
📒 Files selected for processing (5)
  • Assets/Tests/Editor/NativeCliInstallerTests.cs
  • Packages/src/Cli~/internal/uninstall/command.go
  • Packages/src/Cli~/internal/uninstall/command_test.go
  • Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs
  • scripts/test-install-release-filter.sh
🚧 Files skipped from review as they are similar to previous changes (3)
  • Packages/src/Cli~/internal/uninstall/command.go
  • Assets/Tests/Editor/NativeCliInstallerTests.cs
  • Packages/src/Cli~/internal/uninstall/command_test.go

Comment thread Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs
Report only the uninstall cleanup condition that is still pending so fallback launcher failures do not claim Windows User PATH cleanup ownership when that wait was intentionally disabled.
@hatayama hatayama merged commit 62a7c88 into v3-beta May 19, 2026
8 checks passed
@hatayama hatayama deleted the codex/restart-windows-cli-uninstall-goal branch May 19, 2026 07:22
@github-actions github-actions Bot mentioned this pull request May 19, 2026
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