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

Autocomplete: When logging in as a new user, feature flags and autocomplete config is not reset #2154

Closed
philipp-spiess opened this issue Dec 6, 2023 · 0 comments · Fixed by #2182
Assignees
Labels
autocomplete bug Something isn't working

Comments

@philipp-spiess
Copy link
Contributor

This can lead to weird situations:

  • Logging in to an enterprise instance still makes requests to dotcom
  • When updating autocomplete settings, we still use the old feature flag values for a different instance
@philipp-spiess philipp-spiess self-assigned this Dec 6, 2023
philipp-spiess added a commit that referenced this issue Dec 8, 2023
Fixes #2181
Fixes #2154

This PR attempts to clean up the auth flow and makes it so that when the
user account is switched, we never accidentely use feature flags of a
wrong server URL and also ensures we reconfigure Autocomplete so it
works.

It does this by removing the relationship between the `AuthProvider` and
a specific webview. This is necessary because:

- With features like Autocomplete, we have things that depend on the
AuthProvider unrelated on wether a web view is enabled or not
- We have more than one web view right now and this doesn't make sense

Instead, we make it so that the web views subscribe to the auth state
like every other service.

In untangling this, the biggest issue was undocumented dependencies into
how events were passed. What I noticed was that the `ContextProvider`
plays a major role in initializing the web views. This is something we
should clean up in a follow up PR (IMO the web views should subscribe to
the ContextProvider and not the other way around).

I also noticed that the `cody.auth.sync` action isn't really necessary
as we can subscribe to the `AuthProvider` via `addChangeListener`
anyways (which most of the services did).

Also: Apparently there were two ways on when we reconfigure the
`Configuration` object. I now made sure both ways at least reconfigure
the same components by extracting a `onConfigurationChange` function
inside `main.ts`.

Two unrelated fixes:

- The code to update the upgrade state when the editor was focused would
also run on enterprise instances causing a bunch of errors in the
console. Fixed.
- Added the remote URL to the completion loggers we can easily see where
the request is sent to.

⚠️ This PR has a potential to break areas depending on auth, so please
take some time to test it. I've been testing it as thorough as I could
though!

## Test plan

The canonical test case was:

- Be signed in to dotcom and have the starcoder-hybrid feature flag
enable
- Now switch account to an enterprise instance 
- Ensure that requests are made to the enterprise instance and that we
do not use starcoder model strings



https://github.com/sourcegraph/cody/assets/458591/ab5a19a6-a906-42e6-8f76-ae61b500e30c



<!-- Required. See
https://docs.sourcegraph.com/dev/background-information/testing_principles.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autocomplete bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant