-
Notifications
You must be signed in to change notification settings - Fork 306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(uninstall): Delete cached credentials during re-install (CODY-1043) #5819
Merged
jamesmcnamara
merged 21 commits into
main
from
jsm/cody-1043-delete-creds-after-uninstall
Oct 16, 2024
Merged
feat(uninstall): Delete cached credentials during re-install (CODY-1043) #5819
jamesmcnamara
merged 21 commits into
main
from
jsm/cody-1043-delete-creds-after-uninstall
Oct 16, 2024
Conversation
This file contains 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
abeatrix
reviewed
Oct 7, 2024
pkukielka
pushed a commit
to sourcegraph/jetbrains
that referenced
this pull request
Oct 15, 2024
) This is the JetBrains half of the [CODY-1043](https://linear.app/sourcegraph/issue/CODY-1043/bug-account-credentials-are-cached-between-installs), the VSC half of it is in [this PR](sourcegraph/cody#5819). For VSCode I was able to write an E2E as there are electron & VSCode test utilities for opening / closing instances of VSC and installing / uninstalling plugins. Does anyone know if we have similar utilities that we could use to write a test for this behavior in JB? The good news is that JB actually has proper APIs for this hook so the complexity is much lower here. ## Test plan I manually tested this by running `CODY_DIR='<my local cody>' ./gradlew -PforceAgentBuild=true buildPlugin` and then opening the plugins menu and installing a plugin from disk. ![image](https://github.com/user-attachments/assets/0b0c7482-9dce-4ce5-8f80-07b60857e1d9) I logged in and then uninstalled from the same menu. After reinstalling, it takes me back to the login screen and all previous endpoints are cleared. When I log back into the same account I see my chat history is removed.
abeatrix
reviewed
Oct 15, 2024
abeatrix
reviewed
Oct 16, 2024
Co-authored-by: Beatrix <68532117+abeatrix@users.noreply.github.com>
abeatrix
approved these changes
Oct 16, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't test the uninstall step since we now have a test that covers it, but i've started the build locally and everything works as expected!
RXminuS
pushed a commit
that referenced
this pull request
Oct 16, 2024
…) (#5819) This is the VSCode half of [CODY-1043](https://linear.app/sourcegraph/issue/CODY-1043/bug-account-credentials-are-cached-between-installs). Unfortunately, we don't have access to any VSCode APIs during the uninstall script; it is just a raw node process that VSCode kicks off the next time VSCode is started after an uninstall (could be an arbitrary time after uninstalling). So the way we collected telemetry was to create files in the deactivation subroutine (which is run anytime the extension is stopped, i.e. when it is deactivated, VSCode is closed, the extension is upgraded or uninstalled) that stored all the config needed to boot telemetry and then we read those resources in the uninstall script. Here we do basically the opposite. The uninstall script is run only when the user explicitly uninstalls the extension and closes and re-opens VSCode (importantly it does not run when the extension is upgraded). We create a marker file in the uninstallation process which we can check when the extension is initializing. When present, we delete the file and perform any cleanup from the previous installation (currently just deleting auth credentials and previous endpoints). This also fixes some existing errors in the uninstall script (it isn't working today because the client capabilities aren't initialized), adds a check to the bundling step for the uninstaller that no vscode dependencies are transitively included (similar to the vscode-shim) and adds a re-install telemetry event. Finally added an E2E test for the uninstall flow. It took forever, but seeing as how it was broken for gosh knows how long and we didn't know indicates that it was necessary. To manually test it: 1. Build the VSIX bundle with `pnpm -C vscode _build:vsix_for_test` 2. Start up VSCode and uninstall the Cody extension (or use the alternate editor, for instance I use VSCode Insiders day to day so I do this testing in VSCode). 3. From the command palette run "Extension: Install from VSIX" and select `vscode/dist/cody.e2e.vsix`. Log into Cody normally. 4. Close and reopen code. Verify that your credentials are cached. 5. Uninstall Cody (click on the gear icon on the extension view and select uninstall). 6. Fully close Code (not just the open window but the whole process) 7. Re-open Code and re-install Cody from VSIX. 8. Verify that credentials are cleared and you are presented with a fresh login screen. deletes auth credentials and endpoint history when the extension is reinstalled --------- Co-authored-by: Beatrix <68532117+abeatrix@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is the VSCode half of CODY-1043.
Unfortunately, we don't have access to any VSCode APIs during the uninstall script; it is just a raw node process that VSCode kicks off the next time VSCode is started after an uninstall (could be an arbitrary time after uninstalling). So the way we collected telemetry was to create files in the deactivation subroutine (which is run anytime the extension is stopped, i.e. when it is deactivated, VSCode is closed, the extension is upgraded or uninstalled) that stored all the config needed to boot telemetry and then we read those resources in the uninstall script.
Here we do basically the opposite. The uninstall script is run only when the user explicitly uninstalls the extension and closes and re-opens VSCode (importantly it does not run when the extension is upgraded). We create a marker file in the uninstallation process which we can check when the extension is initializing. When present, we delete the file and perform any cleanup from the previous installation (currently just deleting auth credentials and previous endpoints).
This also fixes some existing errors in the uninstall script (it isn't working today because the client capabilities aren't initialized), adds a check to the bundling step for the uninstaller that no vscode dependencies are transitively included (similar to the vscode-shim) and adds a re-install telemetry event.
Test plan
Finally added an E2E test for the uninstall flow. It took forever, but seeing as how it was broken for gosh knows how long and we didn't know indicates that it was necessary.
To manually test it:
pnpm -C vscode _build:vsix_for_test
vscode/dist/cody.e2e.vsix
. Log into Cody normally.Changelog
deletes auth credentials and endpoint history when the extension is reinstalled