-
Notifications
You must be signed in to change notification settings - Fork 19
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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) | ||
|
@@ -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( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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?> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a s Dominic said :) |
||
// ============= | ||
// Notifications | ||
// ============= | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} | ||
|
||
@JsonNotification("globalStorage/didChange") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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('"')))) | ||
} | ||
} | ||
} |
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.
btw, I wonder... what if the token expires (or is removed)? 🤔 what will happen (with chat, with autocomplete)?
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 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.
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.
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.