-
Notifications
You must be signed in to change notification settings - Fork 30
Fix auth workflow #2419
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
Fix auth workflow #2419
Conversation
// TODO: it seams that some of the settings changes (like enabling/disabling autocomplete) | ||
// requires agent restart to take effect. |
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.
we don't we call CodyAgentService.getInstance(project).restartAgent(project)
if we need it?
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 noticed calling it from there was causing an exception:
java.lang.RuntimeException: java.lang.InterruptedException
at com.intellij.util.concurrency.SchedulingWrapper.shutdownNow(SchedulingWrapper.java:69)
at com.intellij.util.concurrency.BoundedScheduledExecutorService.shutdownNow(BoundedScheduledExecutorService.java:35)
at com.intellij.util.Alarm.dispose(Alarm.java:63)
at com.intellij.openapi.util.ObjectTree.runWithTrace(ObjectTree.java:129)
at com.intellij.openapi.util.ObjectTree.executeAll(ObjectTree.java:161)
at com.intellij.openapi.util.Disposer.dispose(Disposer.java:262)
at com.intellij.openapi.util.Disposer.dispose(Disposer.java:250)
at com.intellij.ui.jcef.JBCefOsrComponent.removeNotify(JBCefOsrComponent.java:108)
at java.desktop/java.awt.Container.removeNotify(Container.java:2835)
at java.desktop/javax.swing.JComponent.removeNotify(JComponent.java:4858)
at com.intellij.ui.jcef.JBCefBrowser$MyPanel.removeNotify(JBCefBrowser.java:415)
at java.desktop/java.awt.Container.lambda$removeAll$2(Container.java:1326)
at java.desktop/sun.awt.SunToolkit.lambda$performWithTreeLock$1(SunToolkit.java:2139)
at java.desktop/sun.awt.SunToolkit.performOnMainThreadIfNeeded(SunToolkit.java:2164)
at java.desktop/sun.awt.SunToolkit.performWithTreeLock(SunToolkit.java:2137)
at java.desktop/java.awt.Container.removeAll(Container.java:1315)
at com.sourcegraph.cody.CodyToolWindowContent.showView(CodyToolWindowContent.kt:79)
at com.sourcegraph.cody.CodyToolWindowContent.refreshPanelsVisibility(CodyToolWindowContent.kt:71)
at com.sourcegraph.cody.CodyToolWindowContent.setWebviewComponent$Sourcegraph(CodyToolWindowContent.kt:96)
at com.sourcegraph.cody.ui.web.CodyToolWindowContentWebviewHost.reset(WebviewView.kt:49)
at com.sourcegraph.cody.ui.web.WebviewViewManager.reset(WebviewView.kt:79)
at com.sourcegraph.cody.ui.web.WebUIService.reset(WebUIService.kt:44)
at com.sourcegraph.cody.agent.CodyAgentService$stopAgent$1.invoke(CodyAgentService.kt:127)
at com.sourcegraph.cody.agent.CodyAgentService$stopAgent$1.invoke(CodyAgentService.kt:126)
at com.sourcegraph.cody.agent.CodyAgentService.stopAgent$lambda$8(CodyAgentService.kt:126)
at java.base/java.util.concurrent.CompletableFuture$UniCompose.tryFire(CompletableFuture.java:1150)
at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2147)
at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleResponse(RemoteEndpoint.java:223)
at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:204)
at org.eclipse.lsp4j.jsonrpc.TracingMessageConsumer.consume(TracingMessageConsumer.java:119)
at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:185)
at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:97)
at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:114)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.InterruptedException
at java.base/java.util.concurrent.FutureTask.awaitDone(FutureTask.java:418)
at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:190)
at com.intellij.util.concurrency.SchedulingWrapper.shutdownNow(SchedulingWrapper.java:66)
... 39 more
Since this is only needed for a cody advanced settings editor which is currently (after switching to webview) not available from the UI (only as an action) I think we can followup on this later.
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 cannot find and reliable repro steps for the issue we talked about via google meet before.
lgtm
Fixes https://linear.app/sourcegraph/issue/QA-71
Fixes https://linear.app/sourcegraph/issue/QA-105
Fixes https://linear.app/sourcegraph/issue/CODY-3884
Fixes
AccountSettingChangeContext
event and whole flow connected with it (fixed inCodyAccountListModel.kt
)refreshPanelsVisibility
to establish proper stateAll in all, considering amount of bugs there, I'm surprised that there were not even more issues visible.
There is one minor remaining issue:
Sometimes when switching to webview, loging panel from webview is visible for a fraction of second.
I think we can live with it until we will fully switch to webview.
Test plan
Manage Accounts
in Cody status bar and add new account of different tier.Account
tab in the Cody panel and make sure account was switchedSign Out
. Native login panel should appear immediately.Manage Accounts
in Cody status bar and make sure both accounts are still there, but neither is marked as active.