feat: uloop can uninstall its global command from terminal and Settings#1135
Merged
Merged
Conversation
Provide a project-independent native command that removes the installed launcher binary on macOS and schedules Windows self-removal after the running process exits. Keep PATH cleanup manual to avoid removing shared user PATH entries, and refresh checked-in CLI binaries for the new command surface.
Contributor
|
Warning Rate limit exceeded
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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds terminal uninstall support to the uloop CLI launcher. It implements OS-specific uninstall command construction (POSIX shell for macOS, deferred PowerShell for Windows), integrates the uninstall handler into the CLI command dispatcher, updates the editor's NativeCliInstaller to orchestrate uninstall execution, and bumps the version to 3.0.0-beta.8. ChangesUninstall Feature Implementation
Sequence Diagram(s)sequenceDiagram
participant User
participant RunProjectLocal
participant tryHandleUninstallRequest
participant resolveUninstallInstallDir
participant CommandForOS
participant exec.CommandContext
participant stdout_stderr
User->>RunProjectLocal: uloop uninstall
RunProjectLocal->>tryHandleUninstallRequest: args
alt uninstall --help
tryHandleUninstallRequest->>stdout_stderr: print help
else uninstall
tryHandleUninstallRequest->>resolveUninstallInstallDir: detect OS
resolveUninstallInstallDir-->>tryHandleUninstallRequest: install_dir
tryHandleUninstallRequest->>CommandForOS: (goos, install_dir, pid)
alt darwin
CommandForOS-->>tryHandleUninstallRequest: shell rm command
else windows
CommandForOS-->>tryHandleUninstallRequest: powershell deferred
end
tryHandleUninstallRequest->>exec.CommandContext: execute
exec.CommandContext-->>tryHandleUninstallRequest: exit_code
tryHandleUninstallRequest->>stdout_stderr: print result
end
stdout_stderr-->>User: removal confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Route the Settings uninstall action through the installed native CLI so terminal and UI removal use the same command contract. Bump the required CLI contract to 3.0.0-beta.8 because the editor now depends on the uninstall subcommand.
hatayama
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Packages/src/Cli~/internal/uninstall/command_test.go (1)
50-55: ⚡ Quick winAssert the encoded PowerShell payload path, not only the helper output.
This assertion currently validates
windowsDeletionScript(...)directly, so it won’t catch regressions wherecommand.Argsno longer embeds that script correctly. Decode the encoded argument and assertWait-Processand PID there.♻️ Suggested test hardening
import ( + "encoding/base64" "strings" "testing" + "unicode/utf16" ) +func decodePowerShellEncodedCommand(t *testing.T, encoded string) string { + t.Helper() + raw, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + t.Fatalf("failed to decode base64 payload: %v", err) + } + if len(raw)%2 != 0 { + t.Fatalf("invalid UTF-16LE payload length: %d", len(raw)) + } + u16 := make([]uint16, 0, len(raw)/2) + for i := 0; i < len(raw); i += 2 { + u16 = append(u16, uint16(raw[i])|uint16(raw[i+1])<<8) + } + return string(utf16.Decode(u16)) +} + func TestCommandForWindowsSchedulesRemovalAfterCurrentProcessExits(t *testing.T) { @@ - deletionScript := windowsDeletionScript(command.TargetPath, 5678) - for _, expected := range []string{"Wait-Process -Id $ParentPid", "$ParentPid = 5678"} { - if !strings.Contains(deletionScript, expected) { - t.Fatalf("expected %s in deletion script: %s", expected, deletionScript) - } - } + outer := decodePowerShellEncodedCommand(t, command.Args[len(command.Args)-1]) + if !strings.Contains(outer, "$EncodedDeletion = '") { + t.Fatalf("outer script does not contain encoded deletion payload: %s", outer) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/src/Cli`~/internal/uninstall/command_test.go around lines 50 - 55, The test currently asserts substrings in windowsDeletionScript(...) instead of the actual encoded payload passed in command.Args; update the test to extract the encoded PowerShell argument from command.Args (the value passed to -EncodedCommand), decode it using the same encoding PowerShell expects (Base64 of UTF-16LE/Unicode), and then assert that the decoded payload contains "Wait-Process -Id $ParentPid" and "$ParentPid = 5678"; locate and modify the assertions around windowsDeletionScript and command.Args to perform the extraction, decode, and checks so regressions embedding the script into command.Args are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs`:
- Around line 200-205: The cancellation branch in RunCliSetupCommand currently
returns a hardcoded message ("Release CLI installer was canceled.") which is
incorrect when this helper is used for uninstall; change the cancellation result
to use the provided commandDescription (e.g., build the returned
CliInstallResult with $"{commandDescription} was canceled." or equivalent) so
the reported operation matches the invoked action; apply the same change to the
other cancellation branch referenced around the second usage (the other return
that currently uses the hardcoded string) so both uninstall and install report
the correct commandDescription.
---
Nitpick comments:
In `@Packages/src/Cli`~/internal/uninstall/command_test.go:
- Around line 50-55: The test currently asserts substrings in
windowsDeletionScript(...) instead of the actual encoded payload passed in
command.Args; update the test to extract the encoded PowerShell argument from
command.Args (the value passed to -EncodedCommand), decode it using the same
encoding PowerShell expects (Base64 of UTF-16LE/Unicode), and then assert that
the decoded payload contains "Wait-Process -Id $ParentPid" and "$ParentPid =
5678"; locate and modify the assertions around windowsDeletionScript and
command.Args to perform the extraction, decode, and checks so regressions
embedding the script into command.Args are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0036f02-c57b-49b3-8bf8-3bd368d45c84
⛔ Files ignored due to path filters (3)
Packages/src/Cli~/dist/darwin-amd64/uloopis excluded by!**/dist/**and included by nonePackages/src/Cli~/dist/darwin-arm64/uloopis excluded by!**/dist/**and included by nonePackages/src/Cli~/dist/windows-amd64/uloop.exeis excluded by!**/dist/**,!**/*.exeand included by none
📒 Files selected for processing (16)
Assets/Tests/Editor/CliSetupApplicationServiceTests.csAssets/Tests/Editor/NativeCliInstallerTests.csPackages/src/Cli~/contract.jsonPackages/src/Cli~/internal/architecture/architecture_test.goPackages/src/Cli~/internal/cli/command_registry.goPackages/src/Cli~/internal/cli/completion_test.goPackages/src/Cli~/internal/cli/error_envelope.goPackages/src/Cli~/internal/cli/help_test.goPackages/src/Cli~/internal/cli/run.goPackages/src/Cli~/internal/cli/uninstall.goPackages/src/Cli~/internal/cli/uninstall_test.goPackages/src/Cli~/internal/tools/default-tools.jsonPackages/src/Cli~/internal/uninstall/command.goPackages/src/Cli~/internal/uninstall/command_test.goPackages/src/Editor/Domain/CliConstants.csPackages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs">
<violation number="1" location="Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs:230">
P2: The cancellation branch in `RunCliSetupCommand` still returns the hardcoded message `"Release CLI installer was canceled."` instead of using the `commandDescription` parameter. When this shared method is called for uninstall, cancelling will incorrectly report "Release CLI installer was canceled." Use `BuildSentenceSubject(commandDescription)` consistently, e.g.:
```csharp
return new CliInstallResult(false, $"{BuildSentenceSubject(commandDescription)} was canceled.");
```</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
Use the shared setup command description when cancellation happens so editor uninstall no longer reports the release installer path. Add focused coverage for the uninstall cancellation message.
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
1 participant
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.
Summary
uloop uninstallfrom a terminal to remove the globally installed uloop command.User Impact
Unknown command: uninstall, and Settings used a separate editor-side deletion path.Changes
ULOOP_INSTALL_DIRfor custom install directories.3.0.0-beta.8and refresh checked-in native binaries.Verification
go test ./...scripts/check-go-cli.shPackages/src/Cli~/dist/darwin-arm64/uloop compile --project-path /Users/a12115/ghq/hatayama/unity-cli-loopPackages/src/Cli~/dist/darwin-arm64/uloop run-tests --project-path /Users/a12115/ghq/hatayama/unity-cli-loop --test-mode EditMode --filter-type regex --filter-value NativeCliInstallerTests|CliSetupApplicationServiceTestsPackages/src/Cli~/dist/darwin-arm64/uloop uninstall --helpcodex-review --mode branch --parallel-tests "scripts/check-go-cli.sh" v3-betaPR Summary: uloop Global Uninstall Command
Overview
This PR adds a project-independent
uloop uninstallnative dispatcher command that enables users to remove the globally installed uloop launcher from their system. The feature addresses the previous limitation where the terminal dispatcher had no CLI method to uninstall the launcher, reportingUnknown command: uninstallinstead.Core Changes
Go CLI Implementation (
Packages/src/Cli~/)New uninstall package (
internal/uninstall/command.go): Implements OS-specific uninstall command constructionrm -f) to remove the installed binaryCLI command handler (
internal/cli/uninstall.go): Processesuloop uninstallinvocationshelpflagULOOP_INSTALL_DIRenv var or runtime OS detection)uninstall.CommandForOSCommand registry update (
internal/cli/command_registry.go): Registersuninstallas a native command--list-commandsoutputC# Editor Integration (
Packages/src/Editor/)NativeCliInstaller refactoring (
Infrastructure/CLI/NativeCliInstaller.cs):UninstallAsyncto acceptCancellationTokenparameterRunCliSetupCommandmethod used by both install and uninstallINSTALL_DIR_ENVIRONMENT_VARIABLEinto child processBuildCliSetupCommandFailure,BuildCliSetupCommandTimeoutFailure)CLI version requirement (
Domain/CliConstants.cs): UpdatedMINIMUM_REQUIRED_CLI_VERSIONfrom3.0.0-beta.7to3.0.0-beta.8Settings integration: Routes editor uninstall action through the installed native CLI instead of performing inline removal
Contract & Manifest Updates
Packages/src/Cli~/contract.json: BumpedcliVersionto3.0.0-beta.8Packages/src/Cli~/internal/tools/default-tools.json: Updatedversionfield to3.0.0-beta.8Test Coverage
Go Tests
internal/uninstall/command_test.go:internal/cli/completion_test.go: Updated to includeuninstallin expected command listinternal/cli/help_test.go: Updated to verifyuninstallappears in launcher and project-local help outputinternal/cli/uninstall_test.go:TestRunProjectLocalUninstallHelpDoesNotRequireUnityProject: Confirms uninstall help works without active project contextC# Tests
Assets/Tests/Editor/NativeCliInstallerTests.cs: Added two test casesBuildUninstallCommand_OnMacRunsInstalledLauncher(): Verifies macOS uninstall command constructionBuildUninstallCommand_OnWindowsRunsInstalledLauncher(): Verifies Windows uninstall command constructionAssets/Tests/Editor/CliSetupApplicationServiceTests.cs: Renamed test method fromGetMinimumRequiredCliVersion_RequiresSingleBinaryCliRelease()toGetMinimumRequiredCliVersion_RequiresTerminalUninstallCliRelease()and updated assertion to expect version3.0.0-beta.8Architecture
classDiagram class CLI["CLI Entry Point (uloop uninstall)"] class UninstallHandler["cli.RunProjectLocal<br/>(tryHandleUninstallRequest)"] class CommandBuilder["uninstall.CommandForOS<br/>- Options<br/>- Command"] class MacOSImpl["macOS Implementation<br/>shell command with rm -f"] class WindowsImpl["Windows Implementation<br/>PowerShell with Wait-Process"] class EditorIntegration["NativeCliInstaller<br/>(UninstallAsync)"] CLI --> UninstallHandler UninstallHandler --> CommandBuilder CommandBuilder --> MacOSImpl CommandBuilder --> WindowsImpl EditorIntegration --> UninstallHandlerUser Impact
Unknown command: uninstallwith no way to remove the launcher via CLIuloop uninstallto remove the launcherVersion Requirement Change
Editor now requires CLI version
3.0.0-beta.8(previously3.0.0-beta.7) due to dependency on the newuninstallsubcommand.