Skip to content
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

App users will be automatically signed in through dotcom (instead of app) for GA #1966

Closed
taylorsperry opened this issue Nov 29, 2023 · 7 comments

Comments

@taylorsperry
Copy link
Contributor

When app users download the app, the first thing they have to do to get setup is create/login to their Sourcegraph.com account. However, when connecting the app to VS Code, they login to their VS Code extension "through the app."

As part of the work to sunset the app, we have the ability to automatically log the user into their dotcom account instead of their app account in VS Code. This feels like the most seamless experience and nothing changes about the user's dotcom account, but we should include an indication that the login method has been updated.

@dominiccooney
Copy link
Contributor

Technical idea here is look at the origin we're signed in to. If it looks like App (localhost:3900 IIRC) then flip to sourcegraph.com instead, automatically.

Question for @toolmantim is whether we should do this silently, or give some notice that this is happening. My $0.02 is that we're probably talking to App, and App is talking to dotcom, so there's no harm in silently making this switch.

@toolmantim
Copy link
Contributor

Here's the UX outline for how this should work, taking into account embeddings importing too:

  1. Cody for VS Code updates behind the scenes for the user
  2. We silently migrate the auth to dotcom (ensuring Cody App is removed from account switcher history etc too)
  3. We send a progress notification (https://code.visualstudio.com/api/ux-guidelines/notifications#progress-notification) with the words "Importing Cody App embeddings…". This closes automatically on completion.
  4. After embeddings import completes (or if Cody App has no embeddings — doesn't matter if this is near-immediate speed wise), we send an information notification:

Cody App is deprecated and your embeddings have been successfully imported into VS Code. You can now safely move Cody App to the bin.
[Move to Bin] [Learn More]"

The "Move to Bin" button would be primary color, and would be a nice-to-have. "Learn More" would open a docs URL.

How's that sound @dominiccooney @taylorsperry?

@taylorsperry
Copy link
Contributor Author

That sounds fantastic. Thank you, @toolmantim and @dominiccooney !

@dominiccooney
Copy link
Contributor

I have #2099 for this.

It does not do the toasting to delete App yet.

Adds local embeddings support for classic sidebar chats, although the context status indicator says embeddings are not found. I assume it is not worth wiring this up at this point.

There's no App index "import" step or UI, we are just reading App's files directly. I can work on this but error reporting, restarting failed index jobs, etc. is probably more important.

dominiccooney added a commit that referenced this issue Dec 5, 2023
This one is a doozy:

- Removes App as a sign-in option.
- If you're signed into App but you have a dotcom token, we'll try to
use that.
- If you're signed into App, but don't have a dotcom token, you'll have
to sign in again.
- Even if you're using classic sidebar chat, if you had an embeddings
index generated by App, we'll pick it up and use it. However the context
status indicator will indicate you have no embeddings. (We could plumb
this in, but it is going away very soon.)
- No more toasts or promotions to set up App.

Leaves the LocalAppDetector so we can delete App/toast you to delete App
soon.

Part of #1966.

## Test plan

**Setup**

The big secret to testing this is recreating the "signed in to App"
state, now that all the sign in options are gone. You do this by abusing
the Enterprise sign-in options:

1. Install Cody App and log into it if you haven't got it already.
2. Sign into dotcom, which caches a token used for the first test below.
3. In VScode, Cmd-Shift-P, Cody: Switch Account... and enter
`http://localhost:3080` which is App's URL.
4. cat ~/Library/Application\ Support/com.sourcegraph.cody/app.json and
smurf the token.
5. At this point, you should be signed in to App. You can verify this by
chatting, quitting app, chatting again and seeing you get a connection
error message.

**Next sign in**

1. Simply reload the window.
2. You should be signed in to dotcom with the token you had been using
at the start of the previous step. You can verify this by chatting with
App closed.
3. Check that Cody App doesn't appear in any of the account switcher or
sign in menus.

**Signed into App, no dotcom token**

Assuming you're still signed in to dotcom:

1. Cmd Shift P, Cody: Sign out.
4. Sign into App using the leetcode techniques above.
5. Reload the window.

You should be dumped at the login pane.

**Signed into App, cached invalid dotcom token**

1. Create a new dotcom auth token at
https://sourcegraph.com/user/settings/tokens/new/callback
2. Sign in to http://sourcegraph.com using that token
3. Cmd Shift P, Cody: Switch Account... and follow the "setup"
instructions above to sign in to app.
4. You are now signed in to App, with a *valid* cached dotcom token.
Switch back to the Sourcegraph token management UI and revoke the token.
5. Reload the window. This encounters a *long* (60s?) pause, but
eventually the login fails and you are dumped back to the login screen
and can log in to get a fresh token.
@dominiccooney
Copy link
Contributor

@taylorsperry , @kalanchan we will automatically log people into dotcom where possible. Worst case they will see the login panel and have to hit login again.

Can we move the toast for removing App to post-GA?

@taylorsperry
Copy link
Contributor Author

The toast is just the reminder that users should now go ahead and delete the app, correct? I think that's fine to wait until post-GA unless there are (near-term, high-likelihood) risks we need to consider around leaving the app on a user's desktop.

Copy link

This issue is marked as stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed automatically in 5 days.

@github-actions github-actions bot added the Stale label Feb 12, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants