From 03029eea47874bf9699053c1ec51478963fe9ce9 Mon Sep 17 00:00:00 2001 From: kirillk Date: Tue, 2 Jun 2026 13:56:23 -0400 Subject: [PATCH] fix(kilo-jetbrains): surface model settings save failures --- .../ai/kilocode/client/KiloNotifications.kt | 19 ++ .../ai/kilocode/client/app/KiloAppService.kt | 8 + .../settings/models/ModelsSettingsUi.kt | 27 ++- .../client/settings/ui/SettingsPanel.kt | 6 + .../settings/ui/SettingsProgressOverlay.kt | 37 +++- .../kotlin/ai/kilocode/client/ui/UiStyle.kt | 12 ++ .../resources/kilo.jetbrains.frontend.xml | 5 + .../resources/messages/KiloBundle.properties | 2 + .../settings/models/ModelsSettingsUiTest.kt | 202 ++++++++++++++++++ .../client/settings/ui/SettingsRowsTest.kt | 63 +++++- .../kilocode/client/testing/FakeAppRpcApi.kt | 5 + 11 files changed, 369 insertions(+), 17 deletions(-) create mode 100644 packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/KiloNotifications.kt create mode 100644 packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/models/ModelsSettingsUiTest.kt diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/KiloNotifications.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/KiloNotifications.kt new file mode 100644 index 00000000000..c0e2e727704 --- /dev/null +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/KiloNotifications.kt @@ -0,0 +1,19 @@ +package ai.kilocode.client + +import com.intellij.notification.Notification +import com.intellij.notification.NotificationGroupManager +import com.intellij.notification.NotificationType +import com.intellij.openapi.project.ProjectManager + +object KiloNotifications { + private const val GROUP = "Kilo Code" + + fun error(title: String, content: String? = null) { + val project = ProjectManager.getInstance().openProjects.firstOrNull { !it.isDefault } + val notification = NotificationGroupManager.getInstance() + .getNotificationGroup(GROUP) + ?.createNotification(title, content ?: "", NotificationType.ERROR) + ?: Notification(GROUP, title, content ?: "", NotificationType.ERROR) + notification.notify(project) + } +} diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/app/KiloAppService.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/app/KiloAppService.kt index 5d2e9c45db1..38f7a1d8dff 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/app/KiloAppService.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/app/KiloAppService.kt @@ -224,6 +224,14 @@ class KiloAppService internal constructor( null } + fun updateConfigAsync( + patch: ConfigPatchDto, + done: (KiloAppStateDto?) -> Unit, + ): Job = cs.launch { + val state = updateConfig(patch) + done(state) + } + private fun setModelState(state: ModelStateDto) { _models.value = state _favorites.value = state.favorite diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/models/ModelsSettingsUi.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/models/ModelsSettingsUi.kt index 19544d82447..e7c7441ab34 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/models/ModelsSettingsUi.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/models/ModelsSettingsUi.kt @@ -1,5 +1,6 @@ package ai.kilocode.client.settings.models +import ai.kilocode.client.KiloNotifications import ai.kilocode.client.app.KiloAppService import ai.kilocode.client.app.KiloWorkspaceService import ai.kilocode.client.plugin.KiloBundle @@ -24,6 +25,7 @@ import ai.kilocode.rpc.dto.ModelsWorkspaceDto import ai.kilocode.rpc.dto.ProvidersDto import com.intellij.ide.DataManager import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.application.ModalityState import com.intellij.openapi.components.service import com.intellij.openapi.options.Configurable import com.intellij.openapi.options.ConfigurableWithId @@ -67,6 +69,8 @@ internal class ModelsSettingsUi( private var allItems: List = emptyList() private var saving = false private var pending: ModelsDraft? = null + private var saveError: String? = null + private var disposed = false init { setContent(form) @@ -84,6 +88,7 @@ internal class ModelsSettingsUi( fun resetDraft() { checkEdt() draft = baseline + saveError = null if (!saving) clearProgress() sync() } @@ -97,11 +102,15 @@ internal class ModelsSettingsUi( if (patch.values.isEmpty() && patch.agents.isEmpty()) return pending = next saving = true + saveError = null showProgress(KiloBundle.message("settings.models.save.pending")) sync() - cs.launch { - val state = app.updateConfig(patch) - withContext(edt) { + app.updateConfigAsync(patch) { state -> + ApplicationManager.getApplication().invokeLater({ + if (disposed) { + if (state == null) KiloNotifications.error(KiloBundle.message("settings.models.save.failed")) + return@invokeLater + } if (state != null) { appState = state val base = modelsDraft(state.config, agents) @@ -109,17 +118,18 @@ internal class ModelsSettingsUi( draft = next pending = null saving = false + saveError = null clearProgress() sync() - return@withContext + return@invokeLater } baseline = prev draft = next pending = null saving = false - clearProgress() + saveError = KiloBundle.message("settings.models.save.failed") sync() - } + }, ModalityState.any()) } } @@ -164,6 +174,7 @@ internal class ModelsSettingsUi( @RequiresEdt fun dispose() { checkEdt() + disposed = true jobs.forEach { it.cancel() } jobs.clear() cs.cancel() @@ -236,8 +247,11 @@ internal class ModelsSettingsUi( else -> top.hideBanner() } } + val err = saveError if (saving || state == ModelsStatus.SAVING) { showProgress(KiloBundle.message("settings.models.save.pending")) + } else if (err != null) { + showError(err) } else if (state == ModelsStatus.UNAVAILABLE || state == ModelsStatus.LOADING) { showProgress(KiloBundle.message("settings.models.loading")) } else { @@ -346,6 +360,7 @@ internal class ModelsSettingsUi( private fun update(fn: ModelsDraft.() -> ModelsDraft) { checkEdt() draft = draft.fn() + saveError = null sync() } diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/ui/SettingsPanel.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/ui/SettingsPanel.kt index 2e5293a482d..be08e0ebe72 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/ui/SettingsPanel.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/ui/SettingsPanel.kt @@ -43,6 +43,12 @@ internal open class SettingsPanel : LayeredOverlayPanel() { overlay.repaint() } + fun showError(text: String) { + progress.showError(text) + overlay.revalidate() + overlay.repaint() + } + fun clearProgress() { progress.clearProgress() overlay.revalidate() diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/ui/SettingsProgressOverlay.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/ui/SettingsProgressOverlay.kt index cff8c3b95eb..25b7664f3d6 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/ui/SettingsProgressOverlay.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/ui/SettingsProgressOverlay.kt @@ -1,7 +1,6 @@ package ai.kilocode.client.settings.ui import ai.kilocode.client.ui.UiStyle -import com.intellij.codeInsight.hint.HintUtil import com.intellij.ui.components.JBLabel import com.intellij.util.ui.JBUI import java.awt.BorderLayout @@ -11,7 +10,10 @@ import java.awt.RenderingHints import javax.swing.JPanel internal class SettingsProgressOverlay : JPanel(BorderLayout()) { + private enum class Kind { INFO, ERROR } + private var label: JBLabel? = null + private var kind: Kind? = null init { val view = JBLabel() @@ -24,7 +26,19 @@ internal class SettingsProgressOverlay : JPanel(BorderLayout()) { } fun showProgress(text: String) { + show(text, Kind.INFO) + } + + fun showError(text: String) { + show(text, Kind.ERROR) + } + + private fun show(text: String, next: Kind) { val view = requireNotNull(label) + if (kind != next) { + kind = next + syncColors() + } if (view.text != text) view.text = text if (!isVisible) isVisible = true revalidate() @@ -46,19 +60,32 @@ internal class SettingsProgressOverlay : JPanel(BorderLayout()) { } private fun syncColors() { - val hint = HintUtil.getInformationHint() - background = hint.textBackground - foreground = hint.textForeground + val current = kind ?: Kind.INFO + background = when (current) { + Kind.INFO -> UiStyle.Colors.infoOverlayBackground() + Kind.ERROR -> UiStyle.Colors.errorOverlayBackground() + } + foreground = when (current) { + Kind.INFO -> UiStyle.Colors.infoOverlayForeground() + Kind.ERROR -> UiStyle.Colors.errorOverlayForeground() + } label?.foreground = foreground } + private fun borderColor() = when (kind ?: Kind.INFO) { + Kind.INFO -> UiStyle.Colors.infoOverlayBorder() + Kind.ERROR -> UiStyle.Colors.errorOverlayBorder() + } + override fun paintComponent(g: Graphics) { val g2 = g.create() as Graphics2D try { g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON) - g2.color = background val arc = UiStyle.Arc.component() + g2.color = background g2.fillRoundRect(0, 0, width, height, arc, arc) + g2.color = borderColor() + g2.drawRoundRect(0, 0, width - 1, height - 1, arc, arc) } finally { g2.dispose() } diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/ui/UiStyle.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/ui/UiStyle.kt index 3f9b680cf40..575f028e567 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/ui/UiStyle.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/ui/UiStyle.kt @@ -111,6 +111,18 @@ object UiStyle { ?: UIUtil.getContextHelpForeground() } + fun infoOverlayBackground(): Color = JBUI.CurrentTheme.NotificationInfo.backgroundColor() + + fun infoOverlayForeground(): Color = JBUI.CurrentTheme.NotificationInfo.foregroundColor() + + fun infoOverlayBorder(): Color = JBUI.CurrentTheme.NotificationInfo.borderColor() + + fun errorOverlayBackground(): Color = JBUI.CurrentTheme.NotificationError.backgroundColor() + + fun errorOverlayForeground(): Color = JBUI.CurrentTheme.NotificationError.foregroundColor() + + fun errorOverlayBorder(): Color = JBUI.CurrentTheme.NotificationError.borderColor() + internal fun contrast(base: Color, delta: Int): Color { val step = if (bright(base)) -delta else delta return Color( diff --git a/packages/kilo-jetbrains/frontend/src/main/resources/kilo.jetbrains.frontend.xml b/packages/kilo-jetbrains/frontend/src/main/resources/kilo.jetbrains.frontend.xml index 66daeba58a7..c54efa1e297 100644 --- a/packages/kilo-jetbrains/frontend/src/main/resources/kilo.jetbrains.frontend.xml +++ b/packages/kilo-jetbrains/frontend/src/main/resources/kilo.jetbrains.frontend.xml @@ -10,6 +10,11 @@ + + () + ApplicationManager.getApplication().messageBus.connect(testRootDisposable).subscribe( + Notifications.TOPIC, + object : Notifications { + override fun notify(notification: Notification) { + notes.add(notification) + } + }, + ) + project.messageBus.connect(testRootDisposable).subscribe( + Notifications.TOPIC, + object : Notifications { + override fun notify(notification: Notification) { + notes.add(notification) + } + }, + ) + val panel = requireUi() + rpc.configUpdateGate = CompletableDeferred() + rpc.configUpdateError = RuntimeException("save failed") + + edt { + select(panel, "new") + panel.applyDraft() + panel.dispose() + ui = null + } + rpc.configUpdateGate?.complete(Unit) + + flushUntil { + notes.any { it.groupId == "Kilo Code" && it.type == NotificationType.ERROR } + } + assertEquals(1, rpc.configUpdateAttempts) + assertTrue(notes.any { it.title == "Failed to save model settings" }) + } + + private fun providers(): ProvidersDto = ProvidersDto( + providers = listOf( + ProviderDto( + id = "kilo", + name = "Kilo", + models = mapOf( + "old" to ModelDto(id = "old", name = "Old"), + "new" to ModelDto(id = "new", name = "New"), + ), + ), + ), + connected = emptyList(), + defaults = emptyMap(), + ) + + private fun select(panel: ModelsSettingsUi, id: String) { + val picker = components(panel).filterIsInstance().first() + picker.onSelect(ModelPicker.Item(id, id.replaceFirstChar { it.titlecase() }, "kilo", "Kilo")) + } + + private fun requireUi(): ModelsSettingsUi = requireNotNull(ui) + + private fun edt(block: () -> T): T { + var result: T? = null + ApplicationManager.getApplication().invokeAndWait { result = block() } + @Suppress("UNCHECKED_CAST") + return result as T + } + + private fun flushUntil(done: () -> Boolean) = runBlocking { + repeat(20) { + delay(100) + edt { UIUtil.dispatchAllInvocationEvents() } + if (done()) return@runBlocking + } + edt { UIUtil.dispatchAllInvocationEvents() } + assertTrue(done()) + } + + private fun text(root: Container): String { + val out = mutableListOf() + for (comp in components(root)) { + if (!comp.isVisible) continue + when (comp) { + is AbstractButton -> comp.text?.let { out.add(it) } + is JLabel -> comp.text?.let { out.add(it) } + is JTextComponent -> comp.text?.let { out.add(it) } + is JTextField -> comp.text?.let { out.add(it) } + } + } + return out.joinToString("\n") + } + + private fun components(root: Container): List = buildList { + fun visit(comp: java.awt.Component) { + add(comp) + if (comp is Container) comp.components.forEach { visit(it) } + } + visit(root) + } +} diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/ui/SettingsRowsTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/ui/SettingsRowsTest.kt index b707c2c2c01..a491df9fa92 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/ui/SettingsRowsTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/ui/SettingsRowsTest.kt @@ -1,12 +1,13 @@ package ai.kilocode.client.settings.ui +import ai.kilocode.client.ui.UiStyle import com.intellij.testFramework.fixtures.BasePlatformTestCase -import com.intellij.codeInsight.hint.HintUtil import com.intellij.ui.SeparatorComponent import com.intellij.ui.components.JBLabel -import ai.kilocode.client.ui.UiStyle +import java.awt.Color import java.awt.Container import java.awt.Rectangle +import java.awt.image.BufferedImage import javax.swing.AbstractButton import javax.swing.JButton import javax.swing.JComponent @@ -112,14 +113,64 @@ class SettingsRowsTest : BasePlatformTestCase() { fun `test settings progress overlay uses information colors`() { val panel = SettingsPanel() - val hint = HintUtil.getInformationHint() panel.showProgress("Loading") val label = components(panel.progress).filterIsInstance().single { it.text == "Loading" } - assertEquals(hint.textBackground, panel.progress.background) - assertEquals(hint.textForeground, panel.progress.foreground) - assertEquals(hint.textForeground, label.foreground) + assertEquals(UiStyle.Colors.infoOverlayBackground(), panel.progress.background) + assertEquals(UiStyle.Colors.infoOverlayForeground(), panel.progress.foreground) + assertEquals(UiStyle.Colors.infoOverlayForeground(), label.foreground) + } + + fun `test settings progress overlay paints styled background`() { + val panel = SettingsPanel() + + panel.showProgress("Saving") + panel.progress.setSize(panel.progress.preferredSize) + panel.progress.doLayout() + + assertEquals(UiStyle.Colors.infoOverlayBackground().rgb, paint(panel.progress, UiStyle.Gap.lg(), panel.progress.height / 2).rgb) + assertNotSameColor(UiStyle.Colors.bg(), panel.progress.background) + } + + fun `test settings progress overlay can show error`() { + val panel = SettingsPanel() + + panel.showError("Failed to save model settings") + + val label = components(panel.progress).filterIsInstance().single { it.text == "Failed to save model settings" } + assertTrue(panel.progress.isVisible) + assertEquals(UiStyle.Colors.errorOverlayBackground(), panel.progress.background) + assertEquals(UiStyle.Colors.errorOverlayForeground(), panel.progress.foreground) + assertEquals(UiStyle.Colors.errorOverlayForeground(), label.foreground) + } + + fun `test settings progress overlay switches error back to info`() { + val panel = SettingsPanel() + + panel.showError("Failed") + val label = components(panel.progress).filterIsInstance().single { it.text == "Failed" } + panel.showProgress("Saving") + + assertSame(label, components(panel.progress).filterIsInstance().single { it.text == "Saving" }) + assertEquals(UiStyle.Colors.infoOverlayBackground(), panel.progress.background) + assertEquals(UiStyle.Colors.infoOverlayForeground(), panel.progress.foreground) + assertEquals(UiStyle.Colors.infoOverlayForeground(), label.foreground) + } + + private fun paint(component: JComponent, x: Int, y: Int): Color { + val image = BufferedImage(component.width, component.height, BufferedImage.TYPE_INT_ARGB) + val g = image.createGraphics() + try { + component.paint(g) + } finally { + g.dispose() + } + return Color(image.getRGB(x, y), true) + } + + private fun assertNotSameColor(a: Color, b: Color) { + assertFalse("Expected colors to differ: $a", a.rgb == b.rgb) } private fun text(component: JComponent): String = components(component) diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/FakeAppRpcApi.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/FakeAppRpcApi.kt index 783047d5f3d..35d1c8c2123 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/FakeAppRpcApi.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/FakeAppRpcApi.kt @@ -35,6 +35,9 @@ class FakeAppRpcApi : KiloAppRpcApi { val cleared = mutableListOf() val variants = mutableListOf() val configPatches = mutableListOf() + var configUpdateAttempts = 0 + private set + var configUpdateGate: CompletableDeferred? = null var configUpdateError: Exception? = null var connected = false @@ -120,6 +123,8 @@ class FakeAppRpcApi : KiloAppRpcApi { override suspend fun updateConfig(patch: ConfigPatchDto): KiloAppStateDto { assertNotEdt("updateConfig") + configUpdateAttempts += 1 + configUpdateGate?.await() configUpdateError?.let { throw it } configPatches.add(patch) val current = state.value