Connect: Improve docs about dual-mode installations#65267
Conversation
gzdunek
added
no-changelog
github-actions
Bot
requested review from
alexhemard,
avatus,
bl-nero,
charlestp,
flyinghermit,
kimlisa,
kiosion,
mmcallister,
nicholasmarais1158,
ptgott,
r0mant,
rudream,
ryanclark,
tele-lion and
zmb3
|
Amplify deployment status
|
gzdunek
removed request for
alexhemard,
mmcallister,
nicholasmarais1158,
r0mant and
ryanclark
gzdunek
removed request for
bl-nero,
charlestp,
flyinghermit,
kimlisa,
kiosion,
rudream and
tele-lion
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — my previous path correction was addressed, and the remaining nits are already captured as inline comments.
Extended reasoning...
Overview
This PR touches three files: a docs page for Teleport Connect installation/uninstall (teleport-connect.mdx), a Windows-specific uninstall include file (uninstall-teleport-connect-windows.mdx), and a small UI error message update in VnetSliderStep.tsx. The changes add a warning admonition about dual-mode installations, add silent uninstall command examples for Windows, and improve VNet error message wording from "reinstall" to "uninstall and install again".
Security risks
No security-sensitive code is touched. No auth, crypto, permissions, or network logic changes. This is purely documentation and UI text.
Level of scrutiny
Low. All changes are either documentation updates or minor UI string changes. The diff is small and self-contained. The previous significant bug I found (incorrect %LOCALAPPDATA%\Teleport Connect path — missing the Programs subdirectory) has been resolved in this revision.
Other factors
The two remaining nits from the bug hunter (missing admin-privilege caveat for silent per-machine uninstall, and ambiguous "uninstall both" wording in the version-mismatch message) are minor and already attached as inline comments. Neither represents a correctness or security issue — the uninstall commands will still work (UAC will trigger automatically), and the user can figure out the intent from context. The PR is a clear net improvement to documentation consistency and user guidance.
There was a problem hiding this comment.
LGTM — all previous inline comments are resolved, the per-user install path and VNet error message are correct in the current diff.
Extended reasoning...
Overview
This small documentation PR (3 files changed) adds a warning admonition about dual per-user/per-machine installations on Windows, fixes the per-user install path in the uninstall docs, adds silent uninstall commands, and clarifies the VNet error message in VnetSliderStep.tsx.
Security risks
None. These are documentation updates and a UI error-message string change with no code logic involved.
Level of scrutiny
Low. The changes are documentation corrections and a one-line UI text improvement. The electron-builder dual-installation behavior described is accurate, and all three files are straightforward.
Other factors
All three inline comments from prior review runs are marked resolved. Reviewing the current diff: the per-user install path correctly shows %LOCALAPPDATA%\Programs\Teleport Connect throughout; the windows-service-version-mismatch error message now explicitly says "uninstall both Teleport Connect installations"; and the new warning admonition accurately describes the electron-builder bug. The minor UAC-note suggestion for the silent uninstall command was raised as 🟡 in previous runs and resolved by the author — omitting it does not make the documented command incorrect, since the NSIS UAC manifest still handles elevation.
* Connect: Improve docs about dual-mode installations * Fix path * Clarify app uninstalling (cherry picked from commit 328db2c)
* Connect: Improve docs about dual-mode installations * Fix path * Clarify app uninstalling (cherry picked from commit 328db2c)
* Connect: Improve docs about dual-mode installations * Fix path * Clarify app uninstalling
3 participants
While performing the final check on the v18 backport, I noticed that switching (or rather installing the app in a new mode) between per-user and per-machine installations behaves inconsistently.
Initially, I assumed that the most recent installation would simply overwrite the Start Menu shortcut. However, this is only true for users with administrative privileges.
Observed behavior
The per-user installation is automatically removed (after the UAC prompt), leaving only the per-machine instance.
The per-user installation overwrites the shortcut, and the Start Menu points to the per-user instance.
The per-machine installation completes after providing admin credentials via UAC, but the Start Menu shortcut still points to the per-user instance.
The per-user installation overwrites the shortcut, and the Start Menu points to the per-user instance.
This appears to be a bug in electron-builder. In the 2.1 scenario, the installer requires admin credentials, which causes the per-user uninstall step to run in the administrator's context rather than the original user's context. As a result, the per-user installation is not properly removed for the target user, leaving the shortcut (and the per-user app) in place.
Ideally, this should be fixed in electron-builder. However, I think for now it's reasonable to document this behavior in the docs and in the app.
I also added docs on how to uninstall the app from the command line.