mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-19 01:51:21 +08:00
fix(jetbrains): show cancellable provider oauth progress
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@kilocode/kilo-jetbrains": patch
|
||||
---
|
||||
|
||||
Show cancellable OAuth progress in JetBrains provider settings and prevent starting another provider action while one is running.
|
||||
@@ -0,0 +1,189 @@
|
||||
# JetBrains Provider OAuth Progress And Locking Plan
|
||||
|
||||
## Goal
|
||||
|
||||
Replace the generic provider OAuth `Loading providers...` overlay with a shared settings progress UI that can show an updating message, countdown, and cancel button. While any provider operation is in flight, prevent starting another provider operation and render provider rows disabled with no action buttons. If the user cancels OAuth, do not refresh provider state; just close the progress UI and restore the existing provider list.
|
||||
|
||||
## Current State
|
||||
|
||||
- `ProvidersSettingsUi` extends `SettingsPanel`, which extends `SettingsOverlayPanel`.
|
||||
- `SettingsOverlayPanel` owns `SettingsProgressOverlay`, currently text-only with `showProgress(text)`, `showError(text)`, and `clearProgress()`.
|
||||
- Provider `reload`, API-key connect, OAuth, disconnect, enable, and custom save all use one `job`/`request` pipeline in `ProvidersSettingsUi.launch`.
|
||||
- `launch` currently cancels the existing job when a new provider action starts, allowing action replacement rather than blocking concurrent actions.
|
||||
- Provider OAuth currently calls `syncLoading()` before authorize/callback, so the overlay says `Loading providers...` during browser authorization.
|
||||
- `LoggedOutProfileUi` already has the desired user-facing countdown copy pattern: `profile.login.waitingTimed=Waiting for authorization... ({0})` with a cancel button.
|
||||
- Provider rows expose actions through `ProviderListRow.actions`; rendering/hit-testing already uses visible action lists and `row.enabled(action)`.
|
||||
|
||||
## Design
|
||||
|
||||
### Shared Settings Progress UI
|
||||
|
||||
Update the shared settings superclass stack, not just provider UI:
|
||||
|
||||
- Extend `SettingsProgressOverlay` to retain:
|
||||
- one message `JBLabel`
|
||||
- an optional cancel `JButton`
|
||||
- the existing `INFO`/`ERROR` kind
|
||||
- Preserve current APIs:
|
||||
- `showProgress(text: String)` remains text-only and non-cancellable.
|
||||
- `showError(text: String)` remains text-only and non-cancellable.
|
||||
- `clearProgress()` hides overlay and removes/clears cancel action.
|
||||
- Add a cancellable progress API on `SettingsOverlayPanel`, for example:
|
||||
- `showProgress(text: String, cancelText: String, cancel: () -> Unit)`
|
||||
- or `showProgress(SettingsProgress(text, cancelText, cancel))` if a tiny data class reads cleaner.
|
||||
- Add `updateProgress(text: String)` on `SettingsOverlayPanel`/`SettingsProgressOverlay` so countdown ticks can update only the message without resetting the cancel action or rebuilding components.
|
||||
- Keep overlay placement in `SettingsOverlayPanel` unchanged.
|
||||
- Use existing style tokens (`UiStyle.Gap`, overlay colors) and IntelliJ components (`JBLabel`, `JButton`).
|
||||
|
||||
### Provider Operation State
|
||||
|
||||
In `ProvidersSettingsUi`:
|
||||
|
||||
- Replace the current “cancel previous job on every launch” behavior for user actions with single-flight guarding.
|
||||
- Track whether an operation is active, e.g. `private var busy = false` plus the existing `job` and `request` generation.
|
||||
- When `busy` is true:
|
||||
- `connect`, `oauth`, `disconnect`, `enable`, `custom`, and `reload` should return without starting another operation.
|
||||
- `ProviderToolbarAction.update` should disable add/refresh actions.
|
||||
- Provider content should render all rows disabled and with no action buttons.
|
||||
- Keep disposal behavior: `dispose()` still invalidates the request and cancels the current job.
|
||||
|
||||
### Disabled Provider Rows With No Buttons
|
||||
|
||||
Update provider list state in the existing model path:
|
||||
|
||||
- Add a `busy` flag to `ProvidersContent`, default false.
|
||||
- Add `setBusy(busy: Boolean)` or include it in `update(state, busy)`.
|
||||
- When busy changes, call `sync()` so the existing rows are rebuilt.
|
||||
- Prefer the smallest row-level change:
|
||||
- Add `disabled: Boolean = false` to `ProviderListRow`.
|
||||
- Return `false` from `ProviderListRow.enabled(action)` when disabled.
|
||||
- Make `ProviderListRenderer.visibleActions(row, selected)` return `emptyList()` when `row.disabled` is true.
|
||||
- In `providerListRows`, accept an optional `disabledRows: Boolean = false` and pass it into every row.
|
||||
- This keeps all provider names/descriptions visible but removes connect/OAuth/disconnect/enable buttons and makes hit-testing/keyboard primary action no-op.
|
||||
- Also disable the search field and list selection/interaction while busy if practical, but the key requirement is no action buttons and no action execution.
|
||||
|
||||
### OAuth Countdown And Cancel
|
||||
|
||||
In `ProvidersSettingsUi.oauth(provider)`:
|
||||
|
||||
- Do not call `syncLoading()` for OAuth.
|
||||
- Start the operation with a provider-specific message, for example:
|
||||
- Initial authorize request: `Starting OAuth for {0}...`
|
||||
- Callback wait: reuse the profile wording style: `Waiting for authorization... ({0})`
|
||||
- Add bundle strings:
|
||||
- `settings.providers.oauth.starting=Starting OAuth for {0}...`
|
||||
- `settings.providers.oauth.waitingTimed=Waiting for authorization... ({0})`
|
||||
- `settings.providers.oauth.cancel=Cancel`
|
||||
- optionally `settings.providers.oauth.failed=OAuth failed` if needed for user-facing errors.
|
||||
- Start a Swing `Timer(1000)` only while waiting for callback.
|
||||
- Countdown source:
|
||||
- Use the provider service OAuth timeout as the expiry target if exposed as an internal constant, or duplicate no magic by defining a local provider UI constant matching the actual 90-second frontend timeout.
|
||||
- Display `m:ss`, matching `LoggedOutProfileUi.syncTime()`.
|
||||
- Flow:
|
||||
- Set busy before starting authorize.
|
||||
- Show cancellable progress with `Starting OAuth for provider.name...` and a cancel callback.
|
||||
- Run `authorize` in the coroutine.
|
||||
- On EDT after authorize returns, check request is still active; open browser if `ready.url` is present; for `method == "code"`, show the code input dialog.
|
||||
- If the code dialog is cancelled or returns blank when code is required, treat it as user cancellation: stop timer, clear progress, restore busy false, and do not call callback or reload.
|
||||
- Before `callback`, start/restart the timer and update progress to `Waiting for authorization... (1:30)` with the same cancel action.
|
||||
- On success, apply returned provider state as today.
|
||||
- On failure, show error and restore provider actions.
|
||||
- On cancel, cancel the job and clear progress without calling `apply`, `state`, workspace reload, or profile refresh.
|
||||
- Make cancel idempotent:
|
||||
- Increment `request` or mark the current request cancelled.
|
||||
- Cancel `job`.
|
||||
- Stop the countdown timer.
|
||||
- Set `busy = false`.
|
||||
- Clear the progress overlay.
|
||||
- Refresh content from existing `state` only by re-rendering with `busy = false`, not by fetching fresh state.
|
||||
|
||||
### Non-OAuth Actions
|
||||
|
||||
For reload/API-key connect/disconnect/enable/custom save:
|
||||
|
||||
- Keep the generic progress text unless a better action-specific string already exists.
|
||||
- Use the same `busy` guard so no action can start while another is running.
|
||||
- On completion/failure, restore `busy = false` and re-render provider rows.
|
||||
- For non-cancelled operation failures, preserve current behavior: error overlay remains visible and state is updated when an action result includes state.
|
||||
- Do not add a cancel button for ordinary provider operations unless it falls out naturally from the shared API; the explicit cancel requirement is for OAuth waiting.
|
||||
|
||||
### Concurrency And Stale Results
|
||||
|
||||
- Keep request-generation checks (`active(id)`) to ignore stale completions.
|
||||
- Do not cancel an existing user action when a new click occurs; ignore new clicks while busy instead.
|
||||
- `reload()` from init should still run normally.
|
||||
- If the user clicks refresh while OAuth is active, it should be disabled/no-op.
|
||||
- If an operation is cancelled by the user, stale coroutine completions must be ignored and must not show an error overlay.
|
||||
- `CancellationException` from user cancellation should not be logged as a failure and should not show an error.
|
||||
|
||||
## Files To Change
|
||||
|
||||
- `packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsProgressOverlay.kt`
|
||||
- Add cancel button support and message update support.
|
||||
- `packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsOverlayPanel.kt`
|
||||
- Expose cancellable progress and message update methods.
|
||||
- `packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/providers/ProvidersSettingsUi.kt`
|
||||
- Add busy/single-flight state.
|
||||
- Add OAuth-specific progress/cancel/countdown flow.
|
||||
- Disable toolbar actions while busy.
|
||||
- Restore existing state without refresh on cancel.
|
||||
- `packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/providers/ProviderListRows.kt`
|
||||
- Add row disabled support and optional disabled-row generation.
|
||||
- `packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/providers/ProviderListRenderer.kt`
|
||||
- Hide all action labels while a row is disabled.
|
||||
- `packages/kilo-jetbrains/frontend/src/main/resources/messages/KiloBundle.properties`
|
||||
- Add OAuth progress/cancel strings.
|
||||
- `packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/base/SettingsRowsTest.kt`
|
||||
- Add shared overlay tests for cancel button retention, message update, and clear behavior.
|
||||
- `packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/providers/ProvidersSettingsUiTest.kt`
|
||||
- Add provider-specific tests for OAuth progress, cancel/no-refresh, and disabled actions.
|
||||
- `packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/FakeProviderRpcApi.kt`
|
||||
- Add OAuth authorize/callback call tracking and optional deferred gates for deterministic tests.
|
||||
|
||||
## Tests To Add
|
||||
|
||||
### Shared Settings Overlay
|
||||
|
||||
- `showProgress(text, cancelText, cancel)` renders one message label and one cancel button.
|
||||
- `updateProgress(text)` updates the retained label and keeps the same cancel button/listener.
|
||||
- Clicking cancel invokes the supplied callback.
|
||||
- `showProgress(text)` after cancellable progress removes/hides the cancel button.
|
||||
- `showError(text)` removes/hides the cancel button and uses error colors.
|
||||
- `clearProgress()` hides overlay and removes/hides cancel button.
|
||||
|
||||
### Provider UI
|
||||
|
||||
- OAuth start shows provider OAuth-specific progress text, not `Loading providers...`.
|
||||
- OAuth callback waiting updates to `Waiting for authorization... (m:ss)` and shows `Cancel`.
|
||||
- While OAuth is active:
|
||||
- toolbar add/refresh actions are disabled or no-op.
|
||||
- provider rows remain visible.
|
||||
- renderer action labels are empty for selected/unselected rows.
|
||||
- mouse/keyboard activation does not invoke another provider action.
|
||||
- Cancel during OAuth:
|
||||
- cancels the current job.
|
||||
- clears progress.
|
||||
- restores provider row actions.
|
||||
- does not call `callback` if cancelled before callback starts.
|
||||
- does not call `state` again beyond the initial load.
|
||||
- does not apply stale completion if the fake deferred completes later.
|
||||
- Operation single-flight:
|
||||
- triggering `reload()` while an OAuth/action is busy does not increment `stateCalls`.
|
||||
- triggering another action while busy does not add to fake RPC action call lists.
|
||||
- Existing stale reload and dispose tests still pass.
|
||||
|
||||
## Verification
|
||||
|
||||
Run the smallest relevant checks from `packages/kilo-jetbrains/`:
|
||||
|
||||
```bash
|
||||
./gradlew :frontend:test --tests ai.kilocode.client.settings.base.SettingsRowsTest --tests ai.kilocode.client.settings.providers.ProvidersSettingsUiTest
|
||||
./gradlew typecheck
|
||||
```
|
||||
|
||||
## Notes
|
||||
|
||||
- Keep all Swing mutations on EDT and annotate new UI methods with `@RequiresEdt`.
|
||||
- Use `javax.swing.Timer` for countdown so ticks run on EDT.
|
||||
- Avoid adding backward-compatible complexity beyond preserving the current public `showProgress(text)`, `showError(text)`, and `clearProgress()` call sites.
|
||||
- Do not refresh provider state on user cancellation; just restore the current in-memory `state` with `busy = false`.
|
||||
+1
-1
@@ -33,7 +33,7 @@ class KiloProviderService internal constructor(
|
||||
companion object {
|
||||
private val LOG = KiloLog.create(KiloProviderService::class.java)
|
||||
private const val RPC_TIMEOUT_MS = 20_000L
|
||||
private const val OAUTH_RPC_TIMEOUT_MS = 90_000L
|
||||
internal const val OAUTH_RPC_TIMEOUT_MS = 90_000L
|
||||
}
|
||||
|
||||
private suspend fun <T> call(name: String, timeoutMs: Long = RPC_TIMEOUT_MS, block: suspend KiloProviderRpcApi.() -> T): T {
|
||||
|
||||
+10
@@ -24,6 +24,16 @@ internal open class SettingsOverlayPanel : LayeredOverlayPanel() {
|
||||
syncOverlay()
|
||||
}
|
||||
|
||||
fun showProgress(text: String, cancelText: String, cancel: () -> Unit) {
|
||||
progress.showProgress(text, cancelText, cancel)
|
||||
syncOverlay()
|
||||
}
|
||||
|
||||
fun updateProgress(text: String) {
|
||||
progress.updateProgress(text)
|
||||
syncOverlay()
|
||||
}
|
||||
|
||||
fun showError(text: String) {
|
||||
progress.showError(text)
|
||||
syncOverlay()
|
||||
|
||||
+36
-5
@@ -7,48 +7,79 @@ import java.awt.BorderLayout
|
||||
import java.awt.Graphics
|
||||
import java.awt.Graphics2D
|
||||
import java.awt.RenderingHints
|
||||
import javax.swing.JButton
|
||||
import javax.swing.JPanel
|
||||
|
||||
internal class SettingsProgressOverlay : JPanel(BorderLayout()) {
|
||||
internal class SettingsProgressOverlay : JPanel(BorderLayout(UiStyle.Gap.md(), 0)) {
|
||||
private enum class Kind { INFO, ERROR }
|
||||
|
||||
private var label: JBLabel? = null
|
||||
private var cancel: JButton? = null
|
||||
private var kind: Kind? = null
|
||||
|
||||
init {
|
||||
val view = JBLabel()
|
||||
val button = JButton()
|
||||
label = view
|
||||
cancel = button
|
||||
isOpaque = false
|
||||
border = JBUI.Borders.empty(UiStyle.Gap.lg(), UiStyle.Gap.pad(), UiStyle.Gap.lg(), UiStyle.Gap.pad())
|
||||
add(view, BorderLayout.CENTER)
|
||||
add(button, BorderLayout.EAST)
|
||||
button.isVisible = false
|
||||
isVisible = false
|
||||
syncColors()
|
||||
}
|
||||
|
||||
fun showProgress(text: String) {
|
||||
show(text, Kind.INFO)
|
||||
show(text, Kind.INFO, null, null)
|
||||
}
|
||||
|
||||
fun showProgress(text: String, cancelText: String, cancel: () -> Unit) {
|
||||
show(text, Kind.INFO, cancelText, cancel)
|
||||
}
|
||||
|
||||
fun showError(text: String) {
|
||||
show(text, Kind.ERROR)
|
||||
show(text, Kind.ERROR, null, null)
|
||||
}
|
||||
|
||||
private fun show(text: String, next: Kind) {
|
||||
fun updateProgress(text: String) {
|
||||
val view = requireNotNull(label)
|
||||
if (view.text != text) view.text = text
|
||||
revalidate()
|
||||
repaint()
|
||||
}
|
||||
|
||||
private fun show(text: String, next: Kind, cancelText: String?, action: (() -> Unit)?) {
|
||||
if (kind != next) {
|
||||
kind = next
|
||||
syncColors()
|
||||
}
|
||||
if (view.text != text) view.text = text
|
||||
updateProgress(text)
|
||||
syncCancel(cancelText, action)
|
||||
if (!isVisible) isVisible = true
|
||||
revalidate()
|
||||
repaint()
|
||||
}
|
||||
|
||||
private fun syncCancel(text: String?, action: (() -> Unit)?) {
|
||||
val button = requireNotNull(cancel)
|
||||
button.actionListeners.toList().forEach { button.removeActionListener(it) }
|
||||
if (text == null || action == null) {
|
||||
button.text = ""
|
||||
button.isVisible = false
|
||||
return
|
||||
}
|
||||
button.text = text
|
||||
button.addActionListener { action() }
|
||||
button.isVisible = true
|
||||
}
|
||||
|
||||
fun clearProgress() {
|
||||
val view = requireNotNull(label)
|
||||
if (!isVisible && view.text.isNullOrBlank()) return
|
||||
view.text = ""
|
||||
syncCancel(null, null)
|
||||
isVisible = false
|
||||
revalidate()
|
||||
repaint()
|
||||
|
||||
+1
@@ -77,6 +77,7 @@ internal class ProviderListRenderer(
|
||||
}
|
||||
|
||||
internal fun visibleActions(row: ProviderListRow, selected: Boolean): List<ProviderListAction> {
|
||||
if (row.disabled) return emptyList()
|
||||
if (row.connected) return row.actions.filter { it == ProviderListAction.DISCONNECT }
|
||||
if (!selected) return emptyList()
|
||||
return row.actions
|
||||
|
||||
+6
-5
@@ -17,13 +17,14 @@ internal data class ProviderListRow(
|
||||
val section: String,
|
||||
val actions: List<ProviderListAction>,
|
||||
val connected: Boolean = false,
|
||||
val disabled: Boolean = false,
|
||||
) {
|
||||
val key: String get() = provider.id
|
||||
|
||||
fun enabled(action: ProviderListAction) = action != ProviderListAction.DISCONNECT || provider.source != "env"
|
||||
fun enabled(action: ProviderListAction) = !disabled && (action != ProviderListAction.DISCONNECT || provider.source != "env")
|
||||
}
|
||||
|
||||
internal fun providerListRows(state: ProviderSettingsDto, query: String): List<ProviderListRow> {
|
||||
internal fun providerListRows(state: ProviderSettingsDto, query: String, disabledRows: Boolean = false): List<ProviderListRow> {
|
||||
val q = query.trim()
|
||||
val ids = state.connected.toSet()
|
||||
val disabled = state.disabled.toSet()
|
||||
@@ -45,9 +46,9 @@ internal fun providerListRows(state: ProviderSettingsDto, query: String): List<P
|
||||
.filter { !hiddenProvider(it) }
|
||||
.sortedWith(compareBy<ProviderSettingsProviderDto> { it.name.lowercase() }.thenBy { it.id })
|
||||
val rows = mutableListOf<ProviderListRow>()
|
||||
rows += connected.map { ProviderListRow(it, KiloBundle.message("settings.providers.connected"), providerActions(it, state, disabled), connected = true) }
|
||||
rows += popular.map { ProviderListRow(it, KiloBundle.message("settings.providers.popular"), providerActions(it, state, disabled)) }
|
||||
rows += all.map { ProviderListRow(it, KiloBundle.message("settings.providers.all"), providerActions(it, state, disabled)) }
|
||||
rows += connected.map { ProviderListRow(it, KiloBundle.message("settings.providers.connected"), providerActions(it, state, disabled), connected = true, disabled = disabledRows) }
|
||||
rows += popular.map { ProviderListRow(it, KiloBundle.message("settings.providers.popular"), providerActions(it, state, disabled), disabled = disabledRows) }
|
||||
rows += all.map { ProviderListRow(it, KiloBundle.message("settings.providers.all"), providerActions(it, state, disabled), disabled = disabledRows) }
|
||||
return rows
|
||||
}
|
||||
|
||||
|
||||
+114
-22
@@ -65,6 +65,7 @@ import javax.swing.KeyStroke
|
||||
import javax.swing.ListSelectionModel
|
||||
import javax.swing.event.DocumentEvent
|
||||
import javax.swing.Icon
|
||||
import javax.swing.Timer
|
||||
|
||||
private val edt = Dispatchers.EDT + ModalityState.any().asContextElement()
|
||||
|
||||
@@ -80,17 +81,21 @@ internal class ProvidersSettingsUi(
|
||||
KiloBundle.message("settings.providers.addCustom"),
|
||||
KiloBundle.message("settings.providers.addCustom.description"),
|
||||
AllIcons.General.Add,
|
||||
{ !busy },
|
||||
) { custom() }
|
||||
private val refresh = ProviderToolbarAction(
|
||||
KiloBundle.message("settings.providers.refresh"),
|
||||
KiloBundle.message("settings.providers.refresh.description"),
|
||||
AllIcons.Actions.Refresh,
|
||||
{ !busy },
|
||||
) { reload() }
|
||||
private val view = ProvidersContent(::connect, ::oauth, ::disconnect, ::enable)
|
||||
private var state = ProviderSettingsDto()
|
||||
private var job: Job? = null
|
||||
private var request = 0
|
||||
private var disposed = false
|
||||
private var busy = false
|
||||
private var timer: Timer? = null
|
||||
|
||||
init {
|
||||
content.add(toolbar(), BorderLayout.NORTH)
|
||||
@@ -102,12 +107,12 @@ internal class ProvidersSettingsUi(
|
||||
fun reload() {
|
||||
checkEdt()
|
||||
LOG.info("provider settings ui reload: start dir=$directory")
|
||||
syncLoading()
|
||||
launch("reload") { id ->
|
||||
if (!launch("reload") { id ->
|
||||
val next = service<KiloProviderService>().state(directory)
|
||||
LOG.info("provider settings ui reload: state providers=${next.providers.size} errors=${next.errors.size}")
|
||||
apply(id, next, null)
|
||||
}
|
||||
}) return
|
||||
syncLoading()
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
@@ -124,11 +129,11 @@ internal class ProvidersSettingsUi(
|
||||
if (!dialog.showAndGet()) return
|
||||
val key = dialog.key()
|
||||
val metadata = dialog.metadata()
|
||||
syncLoading()
|
||||
launch("connect provider=${provider.id}") { id ->
|
||||
if (!launch("connect provider=${provider.id}") { id ->
|
||||
val result = service<KiloProviderService>().connect(ProviderConnectDto(directory, provider.id, key, metadata))
|
||||
apply(id, result.state, result.error)
|
||||
}
|
||||
}) return
|
||||
syncLoading()
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
@@ -136,39 +141,50 @@ internal class ProvidersSettingsUi(
|
||||
checkEdt()
|
||||
val methods = state.auth[provider.id].orEmpty().filter { it.type == "oauth" }
|
||||
val method = state.auth[provider.id].orEmpty().indexOf(methods.firstOrNull()).coerceAtLeast(0).toString()
|
||||
syncLoading()
|
||||
launch("authorize provider=${provider.id}") { id ->
|
||||
if (!launch("authorize provider=${provider.id}") { id ->
|
||||
val ready = service<KiloProviderService>().authorize(ProviderOAuthAuthorizeDto(directory, provider.id, method))
|
||||
val code = withContext(edt) {
|
||||
if (!active(id)) return@withContext null
|
||||
ready.url?.let(BrowserUtil::browse)
|
||||
if (ready.method == "code") Messages.showInputDialog(this@ProvidersSettingsUi, ready.instructions ?: "Enter OAuth code", provider.name, null) else null
|
||||
if (ready.method == "code") {
|
||||
val input = Messages.showInputDialog(this@ProvidersSettingsUi, ready.instructions ?: "Enter OAuth code", provider.name, null)
|
||||
if (input.isNullOrBlank()) {
|
||||
cancelOAuth(id)
|
||||
return@withContext null
|
||||
}
|
||||
input
|
||||
} else null
|
||||
}
|
||||
val current = withContext(edt) { active(id) }
|
||||
if (!current) return@launch
|
||||
withContext(edt) { syncOAuthWaiting(id) }
|
||||
val result = service<KiloProviderService>().callback(ProviderOAuthCallbackDto(directory, provider.id, method, code))
|
||||
apply(id, result.state, result.error)
|
||||
}
|
||||
}) return
|
||||
showProgress(
|
||||
KiloBundle.message("settings.providers.oauth.starting", provider.name),
|
||||
KiloBundle.message("settings.providers.oauth.cancel"),
|
||||
) { cancelOAuth(request) }
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
private fun disconnect(provider: ProviderSettingsProviderDto) {
|
||||
checkEdt()
|
||||
syncLoading()
|
||||
launch("disconnect provider=${provider.id}") { id ->
|
||||
if (!launch("disconnect provider=${provider.id}") { id ->
|
||||
val result = service<KiloProviderService>().disconnect(ProviderDisconnectDto(directory, provider.id))
|
||||
apply(id, result.state, result.error)
|
||||
}
|
||||
}) return
|
||||
syncLoading()
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
private fun enable(provider: ProviderSettingsProviderDto) {
|
||||
checkEdt()
|
||||
syncLoading()
|
||||
launch("enable provider=${provider.id}") { id ->
|
||||
if (!launch("enable provider=${provider.id}") { id ->
|
||||
val result = service<KiloProviderService>().enable(ProviderEnableDto(directory, provider.id))
|
||||
apply(id, result.state, result.error)
|
||||
}
|
||||
}) return
|
||||
syncLoading()
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
@@ -177,11 +193,11 @@ internal class ProvidersSettingsUi(
|
||||
val dialog = CustomProviderDialog()
|
||||
if (!dialog.showAndGet()) return
|
||||
val input = dialog.input(directory)
|
||||
syncLoading()
|
||||
launch("save custom provider") { id ->
|
||||
if (!launch("save custom provider") { id ->
|
||||
val result = service<KiloProviderService>().saveCustom(input)
|
||||
apply(id, result.state, result.error)
|
||||
}
|
||||
}) return
|
||||
syncLoading()
|
||||
}
|
||||
|
||||
private fun toolbar(): JComponent {
|
||||
@@ -193,10 +209,11 @@ internal class ProvidersSettingsUi(
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
private fun launch(name: String, block: suspend (Int) -> Unit) {
|
||||
private fun launch(name: String, block: suspend (Int) -> Unit): Boolean {
|
||||
checkEdt()
|
||||
if (busy || disposed) return false
|
||||
val id = ++request
|
||||
job?.cancel()
|
||||
setBusy(true)
|
||||
job = cs.launch {
|
||||
val start = System.currentTimeMillis()
|
||||
LOG.info("provider settings ui $name: coroutine start dir=$directory")
|
||||
@@ -207,6 +224,7 @@ internal class ProvidersSettingsUi(
|
||||
LOG.warn("provider settings ui $name: coroutine timed out durationMs=${System.currentTimeMillis() - start}", e)
|
||||
withContext(edt) {
|
||||
if (!active(id)) return@withContext
|
||||
setBusy(false)
|
||||
showError("${e::class.simpleName}: ${e.message}")
|
||||
}
|
||||
} catch (e: CancellationException) {
|
||||
@@ -216,10 +234,12 @@ internal class ProvidersSettingsUi(
|
||||
LOG.warn("provider settings ui $name: coroutine failed durationMs=${System.currentTimeMillis() - start}", e)
|
||||
withContext(edt) {
|
||||
if (!active(id)) return@withContext
|
||||
setBusy(false)
|
||||
showError("${e::class.simpleName}: ${e.message}")
|
||||
}
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
private suspend fun apply(id: Int, next: ProviderSettingsDto, error: String?) {
|
||||
@@ -227,6 +247,7 @@ internal class ProvidersSettingsUi(
|
||||
if (!active(id)) return@withContext
|
||||
LOG.info("provider settings ui apply: start providers=${next.providers.size} errors=${next.errors.size} message=${error != null}")
|
||||
state = next
|
||||
setBusy(false)
|
||||
view.update(next)
|
||||
val text = error ?: next.errors.joinToString("; ") { it.detail ?: it.resource }.takeIf { it.isNotBlank() }
|
||||
if (text != null) showError(text) else clearProgress()
|
||||
@@ -234,13 +255,66 @@ internal class ProvidersSettingsUi(
|
||||
}
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
private fun syncOAuthWaiting(id: Int) {
|
||||
checkEdt()
|
||||
if (!active(id)) return
|
||||
val expiry = System.currentTimeMillis() + KiloProviderService.OAUTH_RPC_TIMEOUT_MS
|
||||
fun text(): String {
|
||||
val ms = (expiry - System.currentTimeMillis()).coerceAtLeast(0)
|
||||
val remain = ((ms + 999) / 1000).toInt()
|
||||
val min = remain / 60
|
||||
val sec = remain % 60
|
||||
return KiloBundle.message("settings.providers.oauth.waitingTimed", "$min:${sec.toString().padStart(2, '0')}")
|
||||
}
|
||||
stopTimer()
|
||||
showProgress(text(), KiloBundle.message("settings.providers.oauth.cancel")) { cancelOAuth(id) }
|
||||
timer = Timer(1000) {
|
||||
if (!active(id)) {
|
||||
stopTimer()
|
||||
return@Timer
|
||||
}
|
||||
updateProgress(text())
|
||||
}.also { it.start() }
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
private fun cancelOAuth(id: Int) {
|
||||
checkEdt()
|
||||
if (!active(id)) return
|
||||
request++
|
||||
job?.cancel()
|
||||
job = null
|
||||
stopTimer()
|
||||
setBusy(false)
|
||||
clearProgress()
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
private fun stopTimer() {
|
||||
checkEdt()
|
||||
timer?.stop()
|
||||
timer = null
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
private fun setBusy(next: Boolean) {
|
||||
checkEdt()
|
||||
if (busy == next) return
|
||||
busy = next
|
||||
if (!next) stopTimer()
|
||||
view.setBusy(next)
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
override fun dispose() {
|
||||
checkEdt()
|
||||
disposed = true
|
||||
request++
|
||||
stopTimer()
|
||||
job?.cancel()
|
||||
job = null
|
||||
setBusy(false)
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
@@ -269,6 +343,7 @@ internal class ProvidersContent(
|
||||
textEditor.emptyText.text = KiloBundle.message("settings.providers.search")
|
||||
}
|
||||
private var state = ProviderSettingsDto()
|
||||
private var busy = false
|
||||
|
||||
init {
|
||||
list.cellRenderer = ProviderListRenderer(model)
|
||||
@@ -325,10 +400,21 @@ internal class ProvidersContent(
|
||||
ProvidersSettingsUi.LOG.info("provider settings content update: completed rows=${model.size}")
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
fun setBusy(next: Boolean) {
|
||||
checkEdt()
|
||||
if (busy == next) return
|
||||
busy = next
|
||||
search.isEnabled = !next
|
||||
search.textEditor.isEnabled = !next
|
||||
list.isEnabled = !next
|
||||
sync()
|
||||
}
|
||||
|
||||
@RequiresEdt
|
||||
private fun sync(prefer: String? = list.selectedValue?.key, at: Int? = null) {
|
||||
checkEdt()
|
||||
val rows = providerListRows(state, search.text)
|
||||
val rows = providerListRows(state, search.text, disabledRows = busy)
|
||||
model.replaceAll(rows)
|
||||
val idx = at?.let { providerListIndex(rows, it) }?.takeIf { it >= 0 }
|
||||
?: providerListIndex(rows, prefer).takeIf { it >= 0 }
|
||||
@@ -383,13 +469,19 @@ private class ProviderToolbarAction(
|
||||
text: String,
|
||||
description: String,
|
||||
icon: Icon,
|
||||
private val enabled: () -> Boolean,
|
||||
private val action: () -> Unit,
|
||||
) : DumbAwareAction(text, description, icon) {
|
||||
override fun getActionUpdateThread() = ActionUpdateThread.EDT
|
||||
|
||||
override fun actionPerformed(e: AnActionEvent) {
|
||||
if (!enabled()) return
|
||||
action()
|
||||
}
|
||||
|
||||
override fun update(e: AnActionEvent) {
|
||||
e.presentation.isEnabled = enabled()
|
||||
}
|
||||
}
|
||||
|
||||
private class ApiKeyDialog(title: String, method: ProviderAuthMethodDto?) : DialogWrapper(true) {
|
||||
|
||||
@@ -243,6 +243,9 @@ settings.providers.refresh=Refresh
|
||||
settings.providers.refresh.description=Refresh provider settings
|
||||
settings.providers.connect=Connect
|
||||
settings.providers.oauth=OAuth
|
||||
settings.providers.oauth.starting=Starting OAuth for {0}...
|
||||
settings.providers.oauth.waitingTimed=Waiting for authorization... ({0})
|
||||
settings.providers.oauth.cancel=Cancel
|
||||
settings.providers.disconnect=Disconnect
|
||||
settings.providers.enable=Enable
|
||||
settings.providers.note.kilo=Access 500+ AI models
|
||||
|
||||
+41
@@ -161,6 +161,47 @@ class SettingsRowsTest : BasePlatformTestCase() {
|
||||
assertFalse(panel.progress.isVisible)
|
||||
}
|
||||
|
||||
fun `test settings progress overlay retains cancel button across progress updates`() {
|
||||
val panel = SettingsPanel()
|
||||
var calls = 0
|
||||
|
||||
panel.showProgress("Starting", "Cancel") { calls++ }
|
||||
val label = components(panel.progress).filterIsInstance<JBLabel>().single { it.text == "Starting" }
|
||||
val button = components(panel.progress).filterIsInstance<JButton>().single { it.text == "Cancel" }
|
||||
|
||||
panel.updateProgress("Waiting")
|
||||
button.doClick()
|
||||
|
||||
assertSame(label, components(panel.progress).filterIsInstance<JBLabel>().single { it.text == "Waiting" })
|
||||
assertSame(button, components(panel.progress).filterIsInstance<JButton>().single { it.text == "Cancel" })
|
||||
assertEquals(1, calls)
|
||||
}
|
||||
|
||||
fun `test settings progress overlay clears cancel action for text only states`() {
|
||||
val panel = SettingsPanel()
|
||||
var calls = 0
|
||||
|
||||
panel.showProgress("Starting", "Cancel") { calls++ }
|
||||
val button = components(panel.progress).filterIsInstance<JButton>().single { it.text == "Cancel" }
|
||||
panel.showProgress("Loading")
|
||||
|
||||
assertFalse(button.isVisible)
|
||||
button.doClick()
|
||||
assertEquals(0, calls)
|
||||
|
||||
panel.showProgress("Starting", "Cancel") { calls++ }
|
||||
panel.showError("Failed")
|
||||
assertFalse(button.isVisible)
|
||||
button.doClick()
|
||||
assertEquals(0, calls)
|
||||
|
||||
panel.showProgress("Starting", "Cancel") { calls++ }
|
||||
panel.clearProgress()
|
||||
assertFalse(button.isVisible)
|
||||
button.doClick()
|
||||
assertEquals(0, calls)
|
||||
}
|
||||
|
||||
fun `test settings progress overlay uses information colors`() {
|
||||
val panel = SettingsPanel()
|
||||
|
||||
|
||||
+108
-7
@@ -7,6 +7,7 @@ import ai.kilocode.rpc.dto.ModelDto
|
||||
import ai.kilocode.rpc.dto.ProviderAuthMethodDto
|
||||
import ai.kilocode.rpc.dto.ProviderDisconnectDto
|
||||
import ai.kilocode.rpc.dto.ProviderMetadataDto
|
||||
import ai.kilocode.rpc.dto.ProviderOAuthReadyDto
|
||||
import ai.kilocode.rpc.dto.ProviderSettingsDto
|
||||
import ai.kilocode.rpc.dto.ProviderSettingsProviderDto
|
||||
import com.intellij.openapi.application.ApplicationManager
|
||||
@@ -29,10 +30,13 @@ import java.awt.Container
|
||||
import java.awt.Dimension
|
||||
import java.awt.Point
|
||||
import java.awt.Rectangle
|
||||
import java.awt.event.ActionEvent
|
||||
import java.awt.event.KeyEvent
|
||||
import java.awt.image.BufferedImage
|
||||
import javax.swing.JButton
|
||||
import javax.swing.JComponent
|
||||
import javax.swing.JScrollPane
|
||||
import javax.swing.KeyStroke
|
||||
import javax.swing.UIManager
|
||||
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
@@ -335,6 +339,22 @@ class ProvidersSettingsUiTest : BasePlatformTestCase() {
|
||||
}
|
||||
}
|
||||
|
||||
fun `test disabled provider rows hide action labels and hit targets`() {
|
||||
edt {
|
||||
val row = ProviderListRow(provider("cloudflare", "Cloudflare"), "All providers", listOf(ProviderListAction.OAUTH, ProviderListAction.CONNECT), disabled = true)
|
||||
val list = JBList(listOf(row))
|
||||
val bounds = Rectangle(0, 0, 320, 48)
|
||||
val renderer = ProviderListRenderer(com.intellij.ui.CollectionListModel(listOf(row)))
|
||||
|
||||
renderer.getListCellRendererComponent(list, row, 0, true, false)
|
||||
|
||||
assertTrue(ProviderListRenderer.visibleActions(row, selected = true).isEmpty())
|
||||
assertTrue(ProviderListRenderer.actionBounds(list, bounds, row, selected = true).isEmpty())
|
||||
assertNull(ProviderListRenderer.actionAt(list, bounds, Point(300, 24), row, selected = true))
|
||||
assertTrue(renderer.actionTexts().isEmpty())
|
||||
}
|
||||
}
|
||||
|
||||
fun `test renderer uses standard button foreground for actions`() {
|
||||
edt {
|
||||
val row = ProviderListRow(provider("cloudflare", "Cloudflare"), "All providers", listOf(ProviderListAction.OAUTH, ProviderListAction.CONNECT))
|
||||
@@ -452,6 +472,85 @@ class ProvidersSettingsUiTest : BasePlatformTestCase() {
|
||||
}
|
||||
}
|
||||
|
||||
fun `test provider oauth shows starting progress and disables actions while authorizing`() {
|
||||
val ready = CompletableDeferred<ProviderOAuthReadyDto>()
|
||||
val rpc = installProvider(
|
||||
ProviderSettingsDto(
|
||||
providers = listOf(provider("github-copilot", "GitHub Copilot")),
|
||||
auth = mapOf("github-copilot" to listOf(ProviderAuthMethodDto("oauth", "OAuth"))),
|
||||
),
|
||||
)
|
||||
rpc.authorizesReady.add(ready)
|
||||
val panel = edt { createUi() }
|
||||
|
||||
flushUntil { rpc.stateCalls.size == 1 && edt { rows(panel).map { it.key } == listOf("github-copilot") } }
|
||||
edt { triggerPrimary(panel) }
|
||||
flushUntil { rpc.authorizes.size == 1 && edt { text(panel).contains("Starting OAuth for GitHub Copilot") } }
|
||||
|
||||
edt {
|
||||
assertTrue(text(panel).contains("Cancel"))
|
||||
assertTrue(rows(panel).single().disabled)
|
||||
assertTrue(ProviderListRenderer.visibleActions(rows(panel).single(), selected = true).isEmpty())
|
||||
panel.reload()
|
||||
}
|
||||
|
||||
flushUntil { rpc.stateCalls.size == 1 }
|
||||
assertFalse(ready.isCompleted)
|
||||
}
|
||||
|
||||
fun `test provider oauth waiting countdown can be cancelled without refresh`() {
|
||||
val callback = CompletableDeferred<ai.kilocode.rpc.dto.ProviderActionResultDto>()
|
||||
val rpc = installProvider(
|
||||
ProviderSettingsDto(
|
||||
providers = listOf(provider("github-copilot", "GitHub Copilot")),
|
||||
auth = mapOf("github-copilot" to listOf(ProviderAuthMethodDto("oauth", "OAuth"))),
|
||||
),
|
||||
)
|
||||
rpc.ready = ProviderOAuthReadyDto(method = "auto")
|
||||
rpc.callbacksReady.add(callback)
|
||||
val panel = edt { createUi() }
|
||||
|
||||
flushUntil { rpc.stateCalls.size == 1 && edt { rows(panel).map { it.key } == listOf("github-copilot") } }
|
||||
edt { triggerPrimary(panel) }
|
||||
flushUntil { rpc.callbacks.size == 1 && edt { text(panel).contains("Waiting for authorization... (1:30)") } }
|
||||
edt { components(panel).filterIsInstance<JButton>().single { it.text == "Cancel" && it.isVisible }.doClick() }
|
||||
|
||||
flushUntil { edt { !text(panel).contains("Waiting for authorization") && rows(panel).single().disabled.not() } }
|
||||
callback.complete(ai.kilocode.rpc.dto.ProviderActionResultDto(providerState(provider("stale", "Stale"))))
|
||||
flushUntil { callback.isCompleted }
|
||||
|
||||
edt {
|
||||
assertEquals(1, rpc.stateCalls.size)
|
||||
assertEquals(listOf("github-copilot"), rows(panel).map { it.key })
|
||||
assertFalse(text(panel).contains("Cancel"))
|
||||
}
|
||||
}
|
||||
|
||||
fun `test provider oauth cancel before authorize completion skips callback`() {
|
||||
val ready = CompletableDeferred<ProviderOAuthReadyDto>()
|
||||
val rpc = installProvider(
|
||||
ProviderSettingsDto(
|
||||
providers = listOf(provider("github-copilot", "GitHub Copilot")),
|
||||
auth = mapOf("github-copilot" to listOf(ProviderAuthMethodDto("oauth", "OAuth"))),
|
||||
),
|
||||
)
|
||||
rpc.authorizesReady.add(ready)
|
||||
val panel = edt { createUi() }
|
||||
|
||||
flushUntil { rpc.stateCalls.size == 1 && edt { rows(panel).map { it.key } == listOf("github-copilot") } }
|
||||
edt { triggerPrimary(panel) }
|
||||
flushUntil { rpc.authorizes.size == 1 && edt { text(panel).contains("Cancel") } }
|
||||
edt { components(panel).filterIsInstance<JButton>().single { it.text == "Cancel" && it.isVisible }.doClick() }
|
||||
ready.complete(ProviderOAuthReadyDto(method = "auto"))
|
||||
flushUntil { ready.isCompleted }
|
||||
|
||||
edt {
|
||||
assertTrue(rpc.callbacks.isEmpty())
|
||||
assertEquals(1, rpc.stateCalls.size)
|
||||
assertEquals(listOf("github-copilot"), rows(panel).map { it.key })
|
||||
}
|
||||
}
|
||||
|
||||
fun `test provider action failure returns error state`() = runBlocking {
|
||||
val cs = CoroutineScope(SupervisorJob())
|
||||
scope = cs
|
||||
@@ -469,23 +568,19 @@ class ProvidersSettingsUiTest : BasePlatformTestCase() {
|
||||
assertEquals(listOf("/test"), rpc.stateCalls)
|
||||
}
|
||||
|
||||
fun `test stale reload result is ignored after newer reload`() {
|
||||
fun `test reload is ignored while existing reload is pending`() {
|
||||
val first = CompletableDeferred<ProviderSettingsDto>()
|
||||
val second = CompletableDeferred<ProviderSettingsDto>()
|
||||
val rpc = installProvider(ProviderSettingsDto())
|
||||
rpc.states.add(first)
|
||||
rpc.states.add(second)
|
||||
val panel = edt { createUi() }
|
||||
|
||||
flushUntil { rpc.stateCalls.size == 1 }
|
||||
edt { panel.reload() }
|
||||
flushUntil { rpc.stateCalls.size == 2 }
|
||||
second.complete(providerState(provider("new", "New")))
|
||||
flushUntil { edt { rows(panel).map { it.key } == listOf("new") } }
|
||||
flushUntil { rpc.stateCalls.size == 1 }
|
||||
first.complete(providerState(provider("old", "Old")))
|
||||
flushUntil { first.isCompleted }
|
||||
|
||||
edt { assertEquals(listOf("new"), rows(panel).map { it.key }) }
|
||||
edt { assertEquals(listOf("old"), rows(panel).map { it.key }) }
|
||||
}
|
||||
|
||||
fun `test dispose ignores pending reload completion`() {
|
||||
@@ -558,6 +653,12 @@ class ProvidersSettingsUiTest : BasePlatformTestCase() {
|
||||
|
||||
private fun center(rect: Rectangle) = Point(rect.x + rect.width / 2, rect.y + rect.height / 2)
|
||||
|
||||
private fun triggerPrimary(component: JComponent) {
|
||||
val list = list(component)
|
||||
val action = list.getActionForKeyStroke(KeyStroke.getKeyStroke(KeyEvent.VK_ENTER, 0))
|
||||
action.actionPerformed(ActionEvent(list, ActionEvent.ACTION_PERFORMED, "enter"))
|
||||
}
|
||||
|
||||
private fun components(component: java.awt.Component): List<java.awt.Component> {
|
||||
val out = mutableListOf<java.awt.Component>()
|
||||
fun visit(c: java.awt.Component) {
|
||||
|
||||
+10
-1
@@ -22,6 +22,11 @@ class FakeProviderRpcApi : KiloProviderRpcApi {
|
||||
val disconnects = mutableListOf<ProviderDisconnectDto>()
|
||||
val enables = mutableListOf<ProviderEnableDto>()
|
||||
val custom = mutableListOf<CustomProviderSaveDto>()
|
||||
val authorizes = mutableListOf<ProviderOAuthAuthorizeDto>()
|
||||
val callbacks = mutableListOf<ProviderOAuthCallbackDto>()
|
||||
val authorizesReady = ArrayDeque<CompletableDeferred<ProviderOAuthReadyDto>>()
|
||||
val callbacksReady = ArrayDeque<CompletableDeferred<ProviderActionResultDto>>()
|
||||
var ready = ProviderOAuthReadyDto()
|
||||
var disconnectError: Exception? = null
|
||||
|
||||
override suspend fun state(directory: String): ProviderSettingsDto {
|
||||
@@ -39,11 +44,15 @@ class FakeProviderRpcApi : KiloProviderRpcApi {
|
||||
|
||||
override suspend fun authorize(input: ProviderOAuthAuthorizeDto): ProviderOAuthReadyDto {
|
||||
assertNotEdt("provider.authorize")
|
||||
return ProviderOAuthReadyDto()
|
||||
authorizes.add(input)
|
||||
if (authorizesReady.isNotEmpty()) return authorizesReady.removeFirst().await()
|
||||
return ready
|
||||
}
|
||||
|
||||
override suspend fun callback(input: ProviderOAuthCallbackDto): ProviderActionResultDto {
|
||||
assertNotEdt("provider.callback")
|
||||
callbacks.add(input)
|
||||
if (callbacksReady.isNotEmpty()) return callbacksReady.removeFirst().await()
|
||||
return ProviderActionResultDto(state)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user