Skip to content

fix: Native uninstall now removes stale shell PATH setup - #1498

Merged
hatayama merged 3 commits into
v3-betafrom
fix/embed-uninstall-scripts
Jul 4, 2026
Merged

fix: Native uninstall now removes stale shell PATH setup#1498
hatayama merged 3 commits into
v3-betafrom
fix/embed-uninstall-scripts

Conversation

@hatayama

@hatayama hatayama commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • Native uninstall now removes the shell profile PATH block that the macOS installer adds.
  • Native uninstall scripts are stored as standalone embedded resources and checked by CI.

User Impact

  • After uninstalling on macOS, new terminals no longer keep the uloop-managed PATH block pointing at the removed install directory.
  • Future uninstall script changes are easier to review and are covered by script linting before release.

Changes

  • Extracted macOS and Windows uninstall script bodies into embedded files under cli/dispatcher/internal/uninstall/scripts.
  • Added placeholder replacement coverage for POSIX and Windows uninstall templates, including decoding the deferred Windows deletion command to ensure replacement happens before encoding.
  • Added macOS uninstall cleanup for uloop-managed shell profile marker blocks while preserving unrelated profile content.
  • Extended dispatcher release input stamping and release-trigger detection to include embedded uninstall templates.
  • Included embedded dispatcher PowerShell templates in CodeRabbit review coverage without opening review for every repository PowerShell helper.

Verification

  • sh -n cli/dispatcher/internal/uninstall/scripts/uninstall_darwin.sh && sh -n cli/dispatcher/internal/install/scripts/install_darwin.sh
  • cd cli/dispatcher && go test ./internal/uninstall -count=1
  • cd cli/release-automation && go test ./internal/automation -run 'TestReleaseTriggerGuardMatchesEmbeddedDispatcherScripts|TestReleaseTriggerGuardRequiresDispatcherTriggerForInstallerChanges' -count=1
  • scripts/test-stamp-release-inputs.sh
  • cd cli/release-automation && go run ./cmd/check-release-triggers --base origin/v3-beta --head HEAD
  • scripts/check-go-cli.sh

Move the POSIX and Windows uninstall script bodies out of Go string literals so they can be linted and parsed as standalone script files. Extend CI, release-trigger detection, and shared-input stamping to cover the embedded uninstall templates alongside the existing installer templates.
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Uninstall command generation for darwin and Windows is refactored to use embedded, placeholder-based script templates instead of inline generated strings. New uninstall scripts are embedded via go:embed, CI adds syntax validation for them, and release-automation stamping/trigger logic is extended to track the new dispatcher uninstall script files.

Changes

Uninstall script templating

Layer / File(s) Summary
Embedded uninstall script templates
cli/dispatcher/internal/uninstall/scripts.go, cli/dispatcher/internal/uninstall/scripts/uninstall_darwin.sh
Adds an embedded filesystem exposing darwin/Windows uninstall script templates via uninstallScriptTemplate, and a new darwin rm -f script with a {{TARGET_PATH}} placeholder.
Command construction using templates
cli/dispatcher/internal/uninstall/command.go
Replaces inline fmt.Sprintf-built shell/PowerShell scripts with template-driven generation via posixUninstallArgs, posixUninstallScript, windowsLaunchScript, and a rewritten windowsDeletionScript using strings.NewReplacer.
Uninstall command tests
cli/dispatcher/internal/uninstall/command_test.go
Updates existing assertions to check rendered template output and adds tests validating placeholder substitution and quoting for POSIX and Windows scripts.
CI syntax validation for new scripts
.github/workflows/build-and-test.yml
Adds sh -n and PowerShell scriptblock validation for the new darwin and Windows uninstall scripts.
Dispatcher input tracking and stamping
cli/release-automation/internal/automation/release_trigger_guard.go, cli/release-automation/internal/automation/release_trigger_guard_test.go, scripts/stamp-release-inputs.sh, scripts/test-stamp-release-inputs.sh, cli/dispatcher/shared-inputs-stamp.json
Renames/broadens the dispatcher script matcher to isDispatcherScriptInput, extends list_dispatcher_script_inputs to include uninstall scripts, updates the stamped hash, and expands fixture/test coverage for stamping behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CommandForOS
  participant posixUninstallArgs
  participant windowsUninstallArgs
  participant uninstallScriptTemplate

  CommandForOS->>posixUninstallArgs: targetPath (darwin)
  posixUninstallArgs->>uninstallScriptTemplate: load uninstall_darwin.sh
  uninstallScriptTemplate-->>posixUninstallArgs: template text
  posixUninstallArgs-->>CommandForOS: rendered sh args

  CommandForOS->>windowsUninstallArgs: targetPath, currentPID (windows)
  windowsUninstallArgs->>uninstallScriptTemplate: load uninstall_windows_delete.ps1
  windowsUninstallArgs->>uninstallScriptTemplate: load uninstall_windows_launch.ps1
  uninstallScriptTemplate-->>windowsUninstallArgs: rendered scripts
  windowsUninstallArgs-->>CommandForOS: launch command args
Loading

Possibly related PRs

  • hatayama/unity-cli-loop#1462: Also updates dispatcher shared-inputs/trigger logic to recognize dispatcher script inputs in the same stamping machinery.
  • hatayama/unity-cli-loop#1475: Also modifies cli/dispatcher/internal/uninstall/command.go, touching the same uninstall command construction flow.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title matches one real part of the change: macOS native uninstall cleanup that removes the stale PATH block.
Description check ✅ Passed The description is directly about embedded native uninstall scripts, CI validation, and release-input stamping changes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/embed-uninstall-scripts

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.

Add a regression check that the deferred Windows deletion command is encoded after template replacement, and include embedded dispatcher PowerShell templates in CodeRabbit review coverage.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found and verified against the latest diff

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="cli/dispatcher/internal/uninstall/scripts/uninstall_darwin.sh">

<violation number="1" location="cli/dispatcher/internal/uninstall/scripts/uninstall_darwin.sh:4">
P2: Uninstall script doesn't clean up shell PATH entries that the install script adds. The macOS install script adds `$InstallDir` to shell profiles (`.bash_profile`, `.zshrc`, `config.fish`) via marker blocks, but this uninstall script only removes the binary. After uninstall, users are left with stale PATH entries pointing to a non-existent directory. The Windows uninstaller handles the equivalent cleanup (removes the install dir from User PATH), so this looks like an oversight in the macOS script. Consider adding shell profile cleanup logic — iterate over the same profile paths that `install_darwin.sh` writes to and strip the `PathBlockStart`/`PathBlockEnd` blocks.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread cli/dispatcher/internal/uninstall/scripts/uninstall_darwin.sh
Remove the uloop-managed shell profile marker blocks during native macOS uninstall so the POSIX path cleanup matches the Windows User PATH cleanup. Cover the behavior with a temp-home regression test that preserves unrelated profile content.
@hatayama hatayama changed the title chore: Make native uninstall scripts lintable fix: Native uninstall now removes stale shell PATH setup Jul 4, 2026
@hatayama
hatayama merged commit 3450a1e into v3-beta Jul 4, 2026
10 checks passed
@hatayama
hatayama deleted the fix/embed-uninstall-scripts branch July 4, 2026 05:39
@github-actions github-actions Bot mentioned this pull request Jul 11, 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