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

Switch to webview-based sign-in flow #2382

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import com.intellij.testFramework.fixtures.BasePlatformTestCase
import com.intellij.testFramework.runInEdtAndWait
import com.sourcegraph.cody.agent.CodyAgentService
import com.sourcegraph.cody.agent.protocol_generated.ProtocolCodeLens
import com.sourcegraph.cody.config.CodyPersistentAccountsHost
import com.sourcegraph.cody.config.SourcegraphServerPath
import com.sourcegraph.cody.auth.CodyPersistentAccountsHost
import com.sourcegraph.cody.auth.SourcegraphServerPath
import com.sourcegraph.cody.edit.lenses.LensListener
import com.sourcegraph.cody.edit.lenses.LensesService
import com.sourcegraph.cody.edit.lenses.providers.EditAcceptCodeVisionProvider
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/com/sourcegraph/cody/CodyToolWindowFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.intellij.ui.content.ContentFactory;
import com.sourcegraph.cody.config.actions.OpenCodySettingsEditorAction;
import com.sourcegraph.config.ConfigUtil;
import com.sourcegraph.config.OpenPluginSettingsAction;
import org.jetbrains.annotations.NotNull;

public class CodyToolWindowFactory implements ToolWindowFactory, DumbAware {
Expand All @@ -30,7 +29,6 @@ public void createToolWindowContent(@NotNull Project project, @NotNull ToolWindo
content.setPreferredFocusableComponent(null);
toolWindow.getContentManager().addContent(content);
DefaultActionGroup customCodySettings = new DefaultActionGroup();
customCodySettings.add(new OpenPluginSettingsAction("Cody Settings..."));
customCodySettings.add(new OpenCodySettingsEditorAction());
customCodySettings.addSeparator();
toolWindow.setAdditionalGearActions(customCodySettings);
Expand Down
19 changes: 0 additions & 19 deletions src/main/java/com/sourcegraph/cody/api/Promises.java

This file was deleted.

6 changes: 0 additions & 6 deletions src/main/java/com/sourcegraph/cody/chat/ChatUIConstants.java

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,14 @@
import com.intellij.notification.Notifications;
import com.intellij.openapi.actionSystem.AnAction;
import com.intellij.openapi.actionSystem.AnActionEvent;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.wm.ToolWindow;
import com.intellij.openapi.wm.ToolWindowManager;
import com.sourcegraph.Icons;
import com.sourcegraph.cody.CodyToolWindowFactory;
import com.sourcegraph.cody.config.CodyAccount;
import com.sourcegraph.cody.config.CodyAccountManager;
import com.sourcegraph.cody.auth.CodyAccount;
import com.sourcegraph.cody.config.CodyApplicationSettings;
import com.sourcegraph.cody.config.CodyAuthenticationManager;
import com.sourcegraph.cody.initialization.Activity;
import com.sourcegraph.cody.statusbar.CodyManageAccountsAction;
import com.sourcegraph.common.NotificationGroups;
import com.sourcegraph.common.ui.DumbAwareEDTAction;
import org.jetbrains.annotations.NotNull;
Expand All @@ -25,19 +21,8 @@ public class CodyAuthNotificationActivity implements Activity {

@Override
public void runActivity(@NotNull Project project) {
CodyAccount activeAccount = CodyAuthenticationManager.getInstance().getAccount();
CodyAccountManager service =
ApplicationManager.getApplication().getService(CodyAccountManager.class);

if (activeAccount != null) {
String token = service.findCredentials(activeAccount);
if (token == null) {
showMissingTokenNotification();
}
}

if (!CodyApplicationSettings.getInstance().isGetStartedNotificationDismissed()
&& activeAccount == null) {
&& !CodyAccount.Companion.hasActiveAccount()) {
showOpenCodySidebarNotification(project);
}
}
Expand Down Expand Up @@ -79,15 +64,4 @@ public void actionPerformed(@NotNull AnActionEvent anActionEvent) {
notification.addAction(neverShowAgainAction);
Notifications.Bus.notify(notification);
}

private void showMissingTokenNotification() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, I wonder... what if the token expires (or is removed)? 🤔 what will happen (with chat, with autocomplete)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will test this scenario today. In general I think we should then (if user try to call some action explicitly) show cody panel, and it should be showing login panel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, VSCode has various APIs for showing views, we don't implement them. If we need to show the view, we should see if the extension is trying to do it anyway. Maybe we just need to implement a couple of those methods/properties in the VSCode shim.

// Display notification
Notification notification =
new Notification(
NotificationGroups.CODY_AUTH, "Missing access token", "", NotificationType.WARNING);

notification.setIcon(Icons.CodyLogo);
notification.addAction(new CodyManageAccountsAction());
Notifications.Bus.notify(notification);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.intellij.util.ui.JBDimension;
import com.intellij.util.ui.JBUI;
import com.sourcegraph.Icons;
import com.sourcegraph.cody.config.actions.OpenCodySettingsEditorAction;
import javax.swing.*;
import org.jetbrains.annotations.NotNull;

Expand All @@ -16,7 +17,7 @@ public class GoToPluginSettingsButtonFactory {
public static ActionButton createGoToPluginSettingsButton() {
JBDimension actionButtonSize = JBUI.size(22, 22);

AnAction action = new OpenPluginSettingsAction();
AnAction action = new OpenCodySettingsEditorAction();
Presentation presentation = new Presentation("Open Plugin Settings");

ActionButton button =
Expand Down
25 changes: 0 additions & 25 deletions src/main/java/com/sourcegraph/config/OpenPluginSettingsAction.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import com.intellij.ui.components.JBPanelWithEmptyText;
import com.intellij.util.ui.JBUI;
import com.intellij.util.ui.StatusText;
import com.sourcegraph.cody.config.ui.AccountConfigurable;
import com.sourcegraph.cody.config.ui.CodyConfigurable;
import java.awt.*;
import javax.swing.*;
import org.apache.commons.lang.WordUtils;
Expand Down Expand Up @@ -69,9 +69,7 @@ private void refreshUI() {
emptyText.appendLine(
"Click here to configure your Sourcegraph Cody + Code Search settings.",
new SimpleTextAttributes(STYLE_PLAIN, JBUI.CurrentTheme.Link.Foreground.ENABLED),
__ ->
ShowSettingsUtil.getInstance()
.showSettingsDialog(project, AccountConfigurable.class));
__ -> ShowSettingsUtil.getInstance().showSettingsDialog(project, CodyConfigurable.class));

} else if (errorMessage != null) {
String wrappedText = WordUtils.wrap("Error: " + errorMessage, 100);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.intellij.openapi.util.Disposer;
import com.intellij.ui.jcef.JBCefBrowser;
import com.sourcegraph.cody.config.notification.AccountSettingChangeListener;
import com.sourcegraph.cody.config.notification.CodySettingChangeListener;
import com.sourcegraph.config.ThemeUtil;
import javax.swing.*;
Expand All @@ -24,11 +23,6 @@ public SourcegraphJBCefBrowser(@NotNull JSToJavaBridgeRequestHandler requestHand
Disposer.register(this, jsToJavaBridge);
javaToJSBridge = new JavaToJSBridge(this);

requestHandler
.getProject()
.getService(AccountSettingChangeListener.class)
.setJavaToJSBridge(javaToJSBridge);

requestHandler
.getProject()
.getService(CodySettingChangeListener.class)
Expand Down
66 changes: 5 additions & 61 deletions src/main/kotlin/com/sourcegraph/cody/CodyToolWindowContent.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,74 +6,30 @@ import com.intellij.openapi.project.Project
import com.intellij.ui.components.JBLabel
import com.intellij.util.concurrency.annotations.RequiresEdt
import com.intellij.util.ui.UIUtil
import com.sourcegraph.cody.chat.SignInWithSourcegraphPanel
import com.sourcegraph.cody.chat.ui.CodyOnboardingGuidancePanel
import com.sourcegraph.cody.config.CodyAccount
import com.sourcegraph.cody.config.CodyApplicationSettings
import com.sourcegraph.cody.config.CodyAuthenticationManager
import com.sourcegraph.cody.ui.web.WebUIService
import java.awt.CardLayout
import java.awt.GridBagConstraints
import java.awt.GridBagLayout
import java.awt.GridLayout
import javax.swing.JComponent
import javax.swing.JPanel

@Service(Service.Level.PROJECT)
class CodyToolWindowContent(project: Project) {
private val cardLayout = CardLayout()
private val cardPanel = JPanel(cardLayout)
val allContentPanel: JComponent = JPanel(GridLayout(1, 1))
val allContentPanel: JComponent = JPanel(CardLayout())
private val mainPanel = JPanel(GridBagLayout())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were using the cardlayout to swap between the loading, login and onboarding components.

We can't put the webview in a CardLayout because JetBrains remote doesn't render it, so we are swapping the components.

But now there are only two components to swap: Loading and webview. So we don't need this panel anymore.

Let's just have allContentPanel with whatever trivial layout (GridLayout(1,1) etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is weird, because I tried to simplify it at the start but it was not working without CardLayout.
But maybe it was error on my side, I will check again :)

private var webviewPanel: JComponent? = null

init {
cardPanel.add(SignInWithSourcegraphPanel(project), SIGN_IN_PANEL, SIGN_IN_PANEL_INDEX)
val codyOnboardingGuidancePanel = CodyOnboardingGuidancePanel(project)
codyOnboardingGuidancePanel.addMainButtonActionListener {
CodyApplicationSettings.instance.isOnboardingGuidanceDismissed = true
refreshPanelsVisibility()
}
cardPanel.add(codyOnboardingGuidancePanel, ONBOARDING_PANEL, ONBOARDING_PANEL_INDEX)

// Because the webview may be created lazily, populate a placeholder control.
val placeholder = JPanel(GridBagLayout())
val spinnerLabel =
JBLabel("Starting Cody...", Icons.StatusBar.CompletionInProgress, JBLabel.CENTER)
placeholder.add(spinnerLabel, GridBagConstraints())
cardPanel.add(placeholder, LOADING_PANEL, LOADING_PANEL_INDEX)

mainPanel.add(spinnerLabel, GridBagConstraints())
WebUIService.getInstance(project).views.provideCodyToolWindowContent(this)

refreshPanelsVisibility()
}

@RequiresEdt
fun refreshPanelsVisibility() {
val codyAuthenticationManager = CodyAuthenticationManager.getInstance()
if (codyAuthenticationManager.hasNoActiveAccount() ||
codyAuthenticationManager.showInvalidAccessTokenError()) {
cardLayout.show(cardPanel, SIGN_IN_PANEL)
showView(cardPanel)
return
}
val activeAccount = codyAuthenticationManager.account
if (!CodyApplicationSettings.instance.isOnboardingGuidanceDismissed) {
val displayName = activeAccount?.let(CodyAccount::displayName)
cardPanel.getComponent(ONBOARDING_PANEL_INDEX)?.let {
(it as CodyOnboardingGuidancePanel).updateDisplayName(displayName)
}
cardLayout.show(cardPanel, ONBOARDING_PANEL)
showView(cardPanel)
return
}
cardLayout.show(cardPanel, LOADING_PANEL)
showView(webviewPanel ?: cardPanel)
}

// Flips the sidebar view to the specified top level component. We do it this way
// because JetBrains Remote does not display webviews inside a component using
// CardLayout.
private fun showView(component: JComponent) {
val component = webviewPanel ?: mainPanel
if (allContentPanel.components.isEmpty() || allContentPanel.getComponent(0) != component) {
allContentPanel.removeAll()
allContentPanel.add(component)
Expand All @@ -84,22 +40,10 @@ class CodyToolWindowContent(project: Project) {
@RequiresEdt
fun setWebviewComponent(component: JComponent?) {
webviewPanel = component
if (component == null) {
refreshPanelsVisibility()
} else {
showView(component)
}
refreshPanelsVisibility()
}

companion object {
const val ONBOARDING_PANEL = "onboardingPanel"
const val SIGN_IN_PANEL = "signInWithSourcegraphPanel"
const val LOADING_PANEL = "loadingPanel"

const val SIGN_IN_PANEL_INDEX = 0
const val ONBOARDING_PANEL_INDEX = 1
const val LOADING_PANEL_INDEX = 2

var logger = Logger.getInstance(CodyToolWindowContent::class.java)

fun executeOnInstanceIfNotDisposed(
Expand Down
2 changes: 2 additions & 0 deletions src/main/kotlin/com/sourcegraph/cody/agent/CodyAgent.kt
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ private constructor(
extensionConfiguration = ConfigUtil.getAgentConfiguration(project),
capabilities =
ClientCapabilities(
authentication = ClientCapabilities.AuthenticationEnum.Enabled,
edit = ClientCapabilities.EditEnum.Enabled,
editWorkspace = ClientCapabilities.EditWorkspaceEnum.Enabled,
codeLenses = ClientCapabilities.CodeLensesEnum.Enabled,
Expand All @@ -128,6 +129,7 @@ private constructor(
untitledDocuments = ClientCapabilities.UntitledDocumentsEnum.Enabled,
codeActions = ClientCapabilities.CodeActionsEnum.Enabled,
globalState = ClientCapabilities.GlobalStateEnum.`Server-managed`,
secrets = ClientCapabilities.SecretsEnum.`Client-managed`,
webview = ClientCapabilities.WebviewEnum.Native,
webviewNativeConfig =
WebviewNativeConfig(
Expand Down
37 changes: 37 additions & 0 deletions src/main/kotlin/com/sourcegraph/cody/agent/CodyAgentClient.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,19 @@ import com.sourcegraph.cody.agent.protocol.WebviewCreateWebviewPanelParams
import com.sourcegraph.cody.agent.protocol_generated.DebugMessage
import com.sourcegraph.cody.agent.protocol_generated.DisplayCodeLensParams
import com.sourcegraph.cody.agent.protocol_generated.Env_OpenExternalParams
import com.sourcegraph.cody.agent.protocol_generated.GlobalStorage_DidChangeParams
import com.sourcegraph.cody.agent.protocol_generated.Null
import com.sourcegraph.cody.agent.protocol_generated.SaveDialogOptionsParams
import com.sourcegraph.cody.agent.protocol_generated.Secrets_DeleteParams
import com.sourcegraph.cody.agent.protocol_generated.Secrets_GetParams
import com.sourcegraph.cody.agent.protocol_generated.Secrets_StoreParams
import com.sourcegraph.cody.agent.protocol_generated.TextDocumentEditParams
import com.sourcegraph.cody.agent.protocol_generated.TextDocument_ShowParams
import com.sourcegraph.cody.agent.protocol_generated.UntitledTextDocument
import com.sourcegraph.cody.agent.protocol_generated.Window_DidChangeContextParams
import com.sourcegraph.cody.agent.protocol_generated.WorkspaceEditParams
import com.sourcegraph.cody.auth.CodyAccount
import com.sourcegraph.cody.auth.SourcegraphServerPath
import com.sourcegraph.cody.edit.EditService
import com.sourcegraph.cody.edit.lenses.LensesService
import com.sourcegraph.cody.error.CodyConsole
Expand Down Expand Up @@ -123,6 +130,24 @@ class CodyAgentClient(private val project: Project, private val webview: NativeW
}
}

@JsonRequest("secrets/get")
fun secrets_get(params: Secrets_GetParams): CompletableFuture<String?> {
return CompletableFuture.completedFuture(
CodyAccount(SourcegraphServerPath(params.key)).getToken())
}

@JsonRequest("secrets/store")
fun secrets_store(params: Secrets_StoreParams): CompletableFuture<Null?> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nullable null... Why not a nullable nullable null 🤔

CodyAccount(SourcegraphServerPath(params.key)).storeToken(params.value)
return CompletableFuture.completedFuture(null)
}

@JsonRequest("secrets/delete")
fun secrets_delete(params: Secrets_DeleteParams): CompletableFuture<Null?> {
CodyAccount(SourcegraphServerPath(params.key)).storeToken(null)
return CompletableFuture.completedFuture(null)
}

Comment on lines +133 to +150
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a stupid question but... is it safe? are there any security issues? would the secrets be present in the trace logs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We log the secrets in the agent trace, not the log. You have to set an extra environment variable to turn on that level of logging... I think it should be fine.

We already log tokens in that file today... the ExtensionConfiguration message from JetBrains to Agent has the token in it... see build/sourcegraph/cody-agent-trace.json after a debugging session.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a s Dominic said :)

// =============
// Notifications
// =============
Expand Down Expand Up @@ -226,4 +251,16 @@ class CodyAgentClient(private val project: Project, private val webview: NativeW

return saveFileFuture
}

@JsonNotification("window/didChangeContext")
fun window_didChangeContext(params: Window_DidChangeContextParams) {
println(params)
Copy link
Contributor

@dominiccooney dominiccooney Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not spam the console. Info logging it might be fine.

Is this, in fact, important for refreshing the status bar icon? "cody.activated" will be true when the extension thinks it is good to go.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, refreshing status icon bar is part I/ know I missed but I forget to add it at the end.
Thanks, I will add it now!

}

@JsonNotification("globalStorage/didChange")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not sniff global storage for this. We should be listening to the configuration observable and pushing a semantic, application-level message to the IDE.

fun globalStorage_didChange(params: GlobalStorage_DidChangeParams) {
if (params.key == "SOURCEGRAPH_CODY_ENDPOINT") {
CodyAccount.setActiveAccount(CodyAccount(SourcegraphServerPath(params.value.trim('"'))))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ fun Model.displayName(): String = buildString {
append(" by $provider")
}

fun Model.isCodyProOnly(): Boolean = tags.contains("pro")
fun Model.isCodyProOnly(): Boolean = tags?.contains("pro") == true

fun Model.isDeprecated(): Boolean = tags.contains("deprecated")
fun Model.isDeprecated(): Boolean = tags?.contains("deprecated") == true
Loading
Loading