Skip to content

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

Merged
merged 1 commit into from
Oct 8, 2024
Merged

Fix auth workflow #2419

merged 1 commit into from
Oct 8, 2024

Conversation

pkukielka
Copy link
Contributor

@pkukielka pkukielka commented Oct 8, 2024

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

  1. Changes done in the account management panel were not triggering AccountSettingChangeContext event and whole flow connected with it (fixed in CodyAccountListModel.kt)
  2. After webview is ready and we should not switch to it immediately but instead call refreshPanelsVisibility to establish proper state
  3. Change listeners being project services need to be invoked at least once to be initialised and in turn connected to the event bus. Otherwise they will not process any events.

All 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

  1. Start new IJ project.
  2. Wait 20s to make sure no webview login panel will appear
  3. Login to the Cody using native login panel
  4. Webview should be displayed
  5. Restart IDE - you should be still logged in with the same account, webview chat view should appear
  6. Open Account Management and remove account, then click Ok. You should be moved back to the native login panel.
  7. Wait some time to make sure webview login panel won't appear.
  8. Login to the Cody using native login panel
  9. Click Manage Accounts in Cody status bar and add new account of different tier.
  10. Click on the Account tab in the Cody panel and make sure account was switched
  11. Click Sign Out. Native login panel should appear immediately.
  12. Click Manage Accounts in Cody status bar and make sure both accounts are still there, but neither is marked as active.
  13. Mark one of them as active and click OK. Webview chat should be displayed

Comment on lines +18 to +19
// TODO: it seams that some of the settings changes (like enabling/disabling autocomplete)
// requires agent restart to take effect.
Copy link
Contributor

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?

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 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.

Copy link
Contributor

@mkondratek mkondratek left a 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

@pkukielka pkukielka merged commit 7525e9f into main Oct 8, 2024
12 of 13 checks passed
@pkukielka pkukielka deleted the pkukielka/fix-auth-workflow branch October 8, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants