Repository navigation
Uninstall the desktop app from its in-window menu on Linux - #1936
realtonyyoung wants to merge 5 commits into
Conversation
The image goes to the freedesktop.org home Trash; one on another filesystem stays put and the folder holding it is opened instead Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Walkthrough
Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 38.60% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 57 functions across 12 files. (2 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding Linux in-window desktop-app uninstall support. Linked Issues check The PR implements the coding requirements in #1782. Linux shows an in-window uninstall item only for a detected AppImage. The uninstall run invokes the existingkcap uninstallflow for the daemon, l…Out of Scope Changes check The changes stay within #1782. The app-menu plumbing, platform-specific removal logic, trash handling, documentation, and tests support the Linux uninstall path. No unrelated product behavior or unrel…Full details: Linked Issues check
-
Explanation
The PR implements the coding requirements in
#1782. Linux shows an in-window uninstall item only for a detected AppImage. The uninstall run invokes the existingkcap uninstallflow for the daemon, local server, harness integrations, links, and~/.config/kcap.LinuxAppRemovalremoves the staged CLI support files and moves the AppImage to the freedesktop Trash. It reports a manual-removal path when the move fails. The added tests cover the Linux removal, AppImage gating, UI visibility, and uninstall flow.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within
#1782. The app-menu plumbing, platform-specific removal logic, trash handling, documentation, and tests support the Linux uninstall path. No unrelated product behavior or unrelated cleanup appears in the reviewed changes.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 38.60% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 57 functions across 12 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
File.Move copies and deletes across filesystems, which moved an image the Trash should have left in place Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The removal's own tests run on every platform, so they move files with their own rename and leave libc to a Linux-only test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rename(2) replaces its destination, so a file another tool left under the claimed name would have been lost; link(2) refuses instead Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A write that fails after the create was read as a name collision, so a full disk claimed names without end; link then unlink could delete a replacement image between the two calls Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR Summary by QodoAdd in-app AppImage uninstall on Linux
AI Description
Diagram
High-Level Assessment
Files changed (14)
|
Code Review by Qodo
1. A compatibility note describes history
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| } catch (EntryPointNotFoundException) { | ||
| // a libc that predates the wrapper | ||
| } |
There was a problem hiding this comment.
1. A compatibility note describes history 📘 Rule violation ⚙ Maintainability
MoveWithoutReplacing includes an added comment saying the fallback handles a libc that predates the wrapper. That historical wording describes how the implementation evolved rather than the current fallback rationale, so future readers receive change-history instead of a stable explanation.
Agent Prompt
## Issue description
The comment near the libc fallback refers to a library that predates the wrapper, which is historical change narration rather than a description of current behavior.
## Fix Focus Areas
- src/Capacitor.App/Platform/Linux/LinuxAppRemoval.cs[143-145]
## Recommended Fix
Replace the historical wording with a stable explanation that the fallback is used when `renameat2` is unavailable or unsupported, without describing how the wrapper evolved.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| public LinuxAppRemoval(IProcessRunner runner, UserHome home, AppImageInstall install, Func<string, string?> getEnv, TimeProvider time) | ||
| : this(runner, install.Image, AppImageInstall.StagedRoot(getEnv, home), KcapLinkFolders.For(home), | ||
| Path.Combine(AppImageInstall.DataHome(getEnv, home), "Trash"), time) { } |
There was a problem hiding this comment.
2. An old data directory survives uninstall 🐞 Bug ≡ Correctness
LinuxAppRemoval recomputes the staged root from the current XDG_DATA_HOME rather than identifying the directory used when the CLI link was installed. If that variable changed between launches, removal targets the new root and cannot remove the earlier staged CLI or its link, while CLI uninstall deliberately keeps path links for app-side cleanup.
Agent Prompt
## Issue description
A change to `XDG_DATA_HOME` makes uninstall inspect and delete a different staged root from the one an earlier launch used, leaving that root and its command link behind.
## Fix Focus Areas
- src/Capacitor.App/Platform/Linux/LinuxAppRemoval.cs[21-23]
- src/Capacitor.App/Platform/Linux/LinuxAppRemoval.cs[42-55]
- src/Capacitor.App/Platform/Linux/AppImageInstall.cs[15-24]
## Recommended Fix
Retain enough installation ownership information to locate previously staged roots and their links across data-home changes. Remove only roots and links verified as belonging to this AppImage installation, while preserving links to unrelated CLI installations.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (Directory.Exists(_stagedRoot)) Directory.Delete(_stagedRoot, recursive: true); | ||
| } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { | ||
| Console.Error.WriteLine($"kcap: could not remove {_stagedRoot}: {ex.Message}"); | ||
| } |
There was a problem hiding this comment.
3. Uninstall succeeds with the cli copy left 🐞 Bug ≡ Correctness
LinuxAppRemoval.DeleteSupportFiles catches a failure to delete the staged directory and returns normally. If permissions or an I/O error prevent deletion, UninstallRunViewModel still moves the AppImage to the Trash and reports success while the copied CLI and plugin remain.
Agent Prompt
## Issue description
A staged-directory deletion failure is logged but does not affect uninstall status, so the AppImage can be trashed while its copied CLI remains.
## Fix Focus Areas
- src/Capacitor.App/Platform/Linux/LinuxAppRemoval.cs[49-55]
- src/Capacitor.App/ViewModels/UninstallRunViewModel.cs[101-108]
## Recommended Fix
Make failure to delete the staged root visible to the uninstall runner and stop before trashing the image. Present the remaining path and a retry or manual-cleanup instruction rather than reporting the uninstall as complete.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (LinkNative(from, to) != 0) return Marshal.GetLastPInvokeError(); | ||
| if (UnlinkNative(from) == 0) return 0; |
There was a problem hiding this comment.
4. A concurrent image replacement can be lost 🐞 Bug ☼ Reliability
MoveWithoutReplacing falls back to link(from, to) followed by unlink(from), without checking that from still names the inode it linked. On a system lacking the renameat2 operation, replacement of the AppImage at that path between those calls makes uninstall delete the replacement file rather than the image placed in the Trash.
Agent Prompt
## Issue description
The link-and-unlink fallback can unlink a different file if the AppImage path changes between its two operations.
## Fix Focus Areas
- src/Capacitor.App/Platform/Linux/LinuxAppRemoval.cs[139-153]
## Recommended Fix
Do not unlink the source through a pathname after a separate link operation. If a no-replace atomic move is unavailable, leave the image in place and use the existing manual-Trash completion path rather than risking deletion of a replacement.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| public Task<IReadOnlyList<string>> RemoveLinksAsync(CancellationToken ct) { | ||
| var remover = new KcapLinkRemover(_linkFolders, Path.Combine(AppImageInstall.StagedCliDirectory(_stagedRoot), "kcap")); | ||
| foreach (var failure in remover.Remove()) Console.Error.WriteLine($"kcap: could not remove {failure.Path}: {failure.Reason}"); | ||
|
|
||
| return Task.FromResult(remover.Find()); |
There was a problem hiding this comment.
5. Links made while the cli ran from the mount survive uninstall 🐞 Bug ≡ Correctness
LinuxAppRemoval.RemoveLinksAsync checks only for kcap links resolving to <stagedRoot>/bin/kcap, although TakeCliOutOfMount can fall back to the CLI inside the AppImage mount when copying it fails. In that fallback case, setup can create a PATH link into /tmp/.mount_*; app-driven CLI uninstall preserves PATH links, and removal trashes the image without finding the mount-backed link, leaving it dangling and causing harness hooks to report “command not found.”
Agent Prompt
## Issue description
When AppImage CLI staging fails, setup can create a persistent `kcap` link into the temporary mount. Linux uninstall checks only the staged CLI target, preserves PATH links during CLI uninstall, and can trash the image while leaving the mount-backed link dangling.
## Fix Focus Areas
- src/Capacitor.App/Platform/Linux/LinuxAppRemoval.cs[40-47]
- src/Capacitor.Cli.Core/Setup/KcapLinkRemover.cs[1-56]
- src/Capacitor.App/Platform/Linux/AppImageInstall.cs[29-46]
## Recommended Fix
Make uninstall identify and remove links created for the AppImage mount as well as the staged CLI, including links whose mount has disappeared. Inspect the symlink’s raw target so a dangling link can still be matched, and retain the check that protects links owned by other CLI installations. Log removal failures as the existing loop does. Alternatively, prevent setup from installing a persistent link to the temporary mount.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔇 Additional comments: 14
src/Capacitor.App/Platform/Linux/AppImageInstall.cs: 1
15-24: LGTM!src/Capacitor.App/Services/IAppRemoval.cs: 1
15-17: LGTM!test/Capacitor.App.Tests.Unit/LinuxAppRemovalTests.cs: 1
1-212: LGTM!src/Capacitor.App/Services/UninstallMode.cs: 1
1-1: LGTM!Also applies to: 26-33
src/Capacitor.App/Views/AppMenu.cs: 1
17-17: LGTM!Also applies to: 24-24, 48-53
src/Capacitor.App/ViewModels/MainWindowViewModel.cs: 1
215-223: LGTM!Also applies to: 351-351, 384-389
src/Capacitor.App/Views/SessionRailView.axaml: 1
376-380: LGTM!src/Capacitor.App/ViewModels/UninstallRunViewModel.cs: 1
46-46: LGTM!Also applies to: 120-120
test/Capacitor.App.Tests.Unit/MainWindowSmokeTests.cs: 1
1476-1476: LGTM!Also applies to: 1485-1490, 1510-1517, 1528-1533
test/Capacitor.App.Tests.Unit/UninstallModeTests.cs: 1
2-2: LGTM!Also applies to: 58-74
test/Capacitor.App.Tests.Unit/UninstallRunViewModelTests.cs: 1
115-116: LGTM!README.md: 1
114-114: LGTM!src/Capacitor.App/App.axaml.cs: 1
935-939: 🩺 Stability & AvailabilityEnsure the uninstall relaunch uses a persistent executable.
RelaunchForUninstallAsyncregistersProcess.Start(start)ondesktop.Exit, whileDesktopHost.NewCopyusesExecutableas the launch path. IfExecutablepoints into the AppImage FUSE mount, the relaunch can lose its executable and assemblies when the original process exits. The uninstall process can then fail beforeRunUninstallModecompletes.Use a persistent executable path for the Linux AppImage relaunch, or otherwise preserve the AppImage before shutdown.
src/Capacitor.App/Platform/Linux/LinuxAppRemoval.cs-103-118: 1
103-118:Minor · Stability & Availability · Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Clean up the claimed
.trashinfowhen the move throws a non-I/O exception.
TrashBundleAsynccatches onlyIOExceptionandUnauthorizedAccessException. The_movedelegate callsLibraryImportP/Invokes. ADllNotFoundExceptionor other runtime exception from these calls skips the cleanup in thecatchblock. The claimedinfo/<name>.trashinfofile then stays in the Trash and names a file that does not exist. The view model catches the exception and shows a failure line, so the orphan entry is the only lasting effect.Track the move result and delete the claimed info file in a
finallyblock. TheTryDeletecall is idempotent, so the existing cleanup on the errno path stays safe.🛡️ Proposed fix
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
b65787c3-0cf6-4280-8e2d-80e5be8e72f3
-
README.mdsrc/Capacitor.App/App.axaml.cssrc/Capacitor.App/Platform/Linux/AppImageInstall.cssrc/Capacitor.App/Platform/Linux/LinuxAppRemoval.cssrc/Capacitor.App/Services/IAppRemoval.cssrc/Capacitor.App/Services/UninstallMode.cssrc/Capacitor.App/ViewModels/MainWindowViewModel.cssrc/Capacitor.App/ViewModels/UninstallRunViewModel.cssrc/Capacitor.App/Views/AppMenu.cssrc/Capacitor.App/Views/SessionRailView.axamltest/Capacitor.App.Tests.Unit/LinuxAppRemovalTests.cstest/Capacitor.App.Tests.Unit/MainWindowSmokeTests.cstest/Capacitor.App.Tests.Unit/UninstallModeTests.cstest/Capacitor.App.Tests.Unit/UninstallRunViewModelTests.cs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
-
Closes #1782 — AI-4148
What & why
Deleting the AppImage left the daemon's systemd user unit, the local server, the
kcaplink (now dangling, so every harness hook failed), the harness integrations and~/.config/kcapbehind. On Linux the app now offers Uninstall kcap… in the in-window menu at the foot of the rail, beside Settings, and runs the same flow as macOS:kcap uninstall, which already removes systemd units, the local server, harness entries, links to itself and the config directory;~/.local/share/kcap/desktop), and the image, which goes to the freedesktop.org home TrashWindows keeps its installer hook and never gets the item.
Where to look
LinuxAppRemoval.TrashBundleAsyncclaims the.trashinfoname withCreateNewbefore moving the image, so two removals never take one name. An image on a different filesystem from the Trash stays where it is: the window then opens its folder and says to move it to the Trash by handIAppRemoval.FinishByHandlets each platform word that last stepVerification
Rail_help_carries_the_app_menu…fails (both rows).trashinfoopened withCreateLinuxAppRemovalTests+UninstallModeTests+UninstallRunViewModelTests+MainWindowSmokeTests+AppMenuBarTests+AppImageInstallTests+ButtonAutomationNameTests+UninstallConfirmationTests+UninstallFactsSourceTestsNot run on Linux: no live AppImage uninstall, and the symlink test is excluded on Windows
🤖 Generated with Claude Code
Summary by CodeRabbit