fix(jetbrains): keep list selection stable across refreshes

ActiveList rebuilt its model with CollectionListModel.replaceAll, which fires an
intervalRemoved with an empty model. The old sync() then fell back to selecting
row 0, and that spurious selection fired onSelect, clobbering the caller's
remembered row. Dropping a dragged worktree and switching away from the Agents
tab both hit this path, so the selection jumped to the first row.

Give ActiveList one capture -> refresh -> restore path: rows expose a stable
identity, the view remembers the selected identities as a sticky anchor written
only by user intent, and restores them after every rebuild with an explicit
absent-row policy. This removes the four hand-rolled restore blocks in
AgentManagerPanel, HistoryPanel, and SettingsInlineListPanel, along with the
AgentManagerPanel.selected mirror and the settings syncing guard.
This commit is contained in:
kirillk
2026-08-21 12:40:52 -04:00
parent 59d1ec8a22
commit 07d2189d31
10 changed files with 346 additions and 70 deletions
@@ -0,0 +1,5 @@
---
"@kilocode/kilo-jetbrains": patch
---
Keep Agent Manager worktree selection stable when reordering worktrees and switching tabs.
@@ -91,7 +91,6 @@ class AgentManagerPanel(
val item = (row as? WorktreeRow)?.dto ?: return@ActiveList
open(item, focus)
},
onSelect = { selectedRow()?.dto?.id?.let { selected = it } },
menu = ActiveListMenu(WorktreeDataKeys.WORKTREE, group, element = { row ->
(row as? WorktreeRow)?.dto?.takeIf { canRename(it) || canDelete(it) || canOpenPr(it) || canOpenDiff(it) }
}),
@@ -100,7 +99,6 @@ class AgentManagerPanel(
onMove = { move -> controller.reorder(move.keys) },
),
)
private var selected: String? = null
private var stats: Map<String, WorktreeStatsDto> = emptyMap()
private var prs: Map<String, WorktreePrDto> = emptyMap()
@@ -150,7 +148,7 @@ class AgentManagerPanel(
}
fun refresh() {
selected = selected ?: currentEditorWorktree()
if (list.selectedKeys().isEmpty()) currentEditorWorktree()?.let { list.select(it, scroll = false) }
controller.reload()
project?.service<WorktreeStatusService>()?.refreshStats()
project?.service<WorktreeStatusService>()?.refreshPr()
@@ -276,9 +274,7 @@ class AgentManagerPanel(
* last row when the removed row was last) and opens it before closing the deleted tab so the
* neighbour becomes the active editor. Deleting a background row leaves the selection untouched.
*
* The active editor is read before close(item) as the ground-truth "shown" signal; the
* `selected` field is unreliable here because the model rebuild in sync() transiently reselects
* row 0 through the list's onSelect hook.
* The active editor is read before close(item) as the ground-truth "shown" signal.
*/
private fun onRemoved(item: WorktreeDto, index: Int) {
if (currentEditorWorktree() == item.id) advance(neighbor(index))
@@ -292,7 +288,6 @@ class AgentManagerPanel(
* the selection somewhere unpredictable.
*/
private fun advance(next: WorktreeDto?) {
selected = next?.id
if (next == null) {
list.clearSelection()
return
@@ -353,7 +348,6 @@ class AgentManagerPanel(
}
private fun sync() {
val key = selected
val current = controller.current?.let { item ->
WorktreeRow(
item,
@@ -382,26 +376,18 @@ class AgentManagerPanel(
},
ActiveListSelection.PreserveNoScroll,
)
if (key != null) {
if (!list.select(key, scroll = false)) list.clearSelection()
return
}
list.clearSelection()
selected = null
}
@RequiresEdt
private fun track(file: VirtualFile?) {
val key = project?.service<WorktreeEditorMatchers>()?.match(file)
if (key != null) {
selected = key
list.select(key, scroll = false)
return
}
// A null active editor is a transient state (e.g. a tab closing during a delete); keep the
// current selection. Only a real, non-worktree editor clears the worktree row selection.
if (file == null) return
selected = null
list.clearSelection()
}
@@ -490,6 +476,7 @@ class AgentManagerPanel(
val current: Boolean = false,
) : ActiveListItem {
override val key: String get() = dto.id
override val identity: Any get() = if (current) "local:${dto.path}" else "worktree:${dto.path}"
override val title: String get() = if (current) dto.branch else WorktreeTitle.text(dto.name, dto.path, pr)
override val description: String get() = WorktreeTitle.fallback(dto.path)
override val tooltip: String? get() = null
@@ -286,19 +286,7 @@ class HistoryPanel(
* list is now empty.
*/
private fun restore(list: ActiveList, rows: List<ActiveListItem>) {
val keys = list.selectedKeys()
val anchor = list.selectedIndex()
list.update(rows, ActiveListSelection.PreserveNoScroll)
val indices = keys.mapNotNull { key -> rows.indexOfFirst { it.key == key }.takeIf { it >= 0 } }.toIntArray()
if (indices.isNotEmpty()) {
list.setSelectionIndices(indices)
return
}
if (keys.isNotEmpty() && rows.isNotEmpty()) {
list.selectIndex(anchor.coerceIn(0, rows.size - 1))
return
}
list.clearSelection()
list.update(rows, ActiveListSelection.Slide)
}
@RequiresEdt
@@ -41,7 +41,6 @@ internal abstract class SettingsInlineListPanel(
private val search = SearchTextField(false)
protected val view = ActiveListView(emptyText, cfg) { key, cellId -> onCell(key, cellId) }
private var toolbar: ActionToolbar? = null
private var syncing = false
@RequiresEdt
protected fun start() {
@@ -51,7 +50,7 @@ internal abstract class SettingsInlineListPanel(
view.setListMinimumSize(JBUI.size(0, minListHeight()))
view.onSelect = {
toolbar?.updateActionsImmediately()
if (!syncing) onSelectionChanged(selectedKeys())
onSelectionChanged(selectedKeys())
}
next(toolbarRow())
gap(UiStyle.Gap.sm())
@@ -81,12 +80,7 @@ internal abstract class SettingsInlineListPanel(
fun setItems(items: List<ActiveListItem>, enabled: Boolean) {
checkEdt()
setEnabled(enabled)
syncing = true
try {
view.update(items, ActiveListSelection.PreserveNoScroll)
} finally {
syncing = false
}
view.update(items, ActiveListSelection.PreserveNoScroll)
toolbar?.updateActionsImmediately()
}
@@ -129,7 +129,7 @@ internal class ActiveList(
@RequiresEdt
fun setSelectionIndices(indices: IntArray) {
view.list.selectedIndices = indices
view.setSelectionIndices(indices)
}
@RequiresEdt
@@ -90,6 +90,11 @@ internal interface ActiveListHitCell {
*/
internal interface ActiveListItem {
val key: String
/**
* Stable identity used to restore selection across refreshes. Defaults to [key]; override when
* the key is not stable for the row's lifetime.
*/
val identity: Any get() = key
val title: String
val note: String? get() = null
val description: String? get() = null
@@ -101,6 +101,9 @@ internal class ActiveListView(
private var hovered = -1
private var heightKey: ActiveListHeightKey? = null
private var drag: Drag? = null
private var anchor: Set<Any> = emptySet()
private var mark = -1
private var restoring = false
// Cursor for the row body; buttons override it on hover via [cursorAt].
private var baseCursor: Cursor = Cursor.getDefaultCursor()
internal var onSelect: (() -> Unit)? = null
@@ -131,6 +134,7 @@ internal class ActiveListView(
list.requestFocusInWindow()
press = null
if (selection(e)) return
rowAt(e.point)?.takeIf { !list.isSelectedIndex(it) }?.let { choose(it, scroll = false) }
val hit = hit(e) ?: return
press = Press(hit.item.key, hit.id ?: return)
}
@@ -196,7 +200,11 @@ internal class ActiveListView(
// Selection gates the hover-revealed action bar, so repaint the hovered row as soon as
// its selection flips instead of waiting for the next mouse move.
if (hover) repaintRow(hovered)
if (!e.valueIsAdjusting) onSelect?.invoke()
if (!restoring && !e.valueIsAdjusting) {
anchor = identities()
mark = list.selectedIndex
onSelect?.invoke()
}
}
list.addFocusListener(object : FocusAdapter() {
override fun focusGained(e: FocusEvent) = list.repaint()
@@ -217,6 +225,8 @@ internal class ActiveListView(
@RequiresEdt
fun clearSelection() {
checkEdt()
anchor = emptySet()
mark = -1
list.clearSelection()
}
@@ -241,7 +251,9 @@ internal class ActiveListView(
@RequiresEdt
fun select(key: String, scroll: Boolean = true): Boolean {
checkEdt()
anchor = setOf(key)
val idx = activeListIndex(model.items, key)
mark = idx
if (idx < 0) return false
choose(idx, scroll)
return true
@@ -250,9 +262,21 @@ internal class ActiveListView(
@RequiresEdt
fun selectIndex(index: Int) {
checkEdt()
val item = model.items.getOrNull(index)
anchor = item?.let { setOf(it.identity) }.orEmpty()
mark = if (item == null) -1 else index
choose(index)
}
@RequiresEdt
fun setSelectionIndices(indices: IntArray) {
checkEdt()
val rows = indices.toList().mapNotNull { model.items.getOrNull(it) }
anchor = rows.map { it.identity }.toSet()
mark = indices.firstOrNull { it in model.items.indices } ?: -1
list.selectedIndices = indices
}
@RequiresEdt
fun setSelectionMode(mode: Int) {
checkEdt()
@@ -297,20 +321,18 @@ internal class ActiveListView(
checkEdt()
if (this.items != items) heightKey = null
this.items = items
val key = when (selection) {
is ActiveListSelection.Key -> selection.key
is ActiveListSelection.Index -> null
val scroll = selection != ActiveListSelection.PreserveNoScroll
when (selection) {
is ActiveListSelection.Key -> {
anchor = setOf(selection.key)
mark = -1
sync(Absent.KEEP, scroll)
}
is ActiveListSelection.Index -> sync(Absent.KEEP, scroll, selection.index, reanchor = true)
ActiveListSelection.Slide -> sync(Absent.SLIDE, scroll)
ActiveListSelection.PreserveNoScroll,
ActiveListSelection.Preserve -> list.selectedValue?.key
ActiveListSelection.Preserve -> sync(Absent.KEEP, scroll)
}
val idx = when (selection) {
is ActiveListSelection.Index -> selection.index
is ActiveListSelection.Key,
ActiveListSelection.PreserveNoScroll,
ActiveListSelection.Preserve,
-> null
}
sync(key, idx, selection != ActiveListSelection.PreserveNoScroll)
}
@RequiresEdt
@@ -340,32 +362,79 @@ internal class ActiveListView(
checkEdt()
if (filter == query) return
filter = query
sync()
sync(Absent.FIRST)
}
@RequiresEdt
private fun sync(prefer: String? = list.selectedValue?.key, at: Int? = null, scroll: Boolean = true) {
private fun sync(absent: Absent = Absent.KEEP, scroll: Boolean = true, at: Int? = null, reanchor: Boolean = false) {
checkEdt()
val q = filter.trim()
val base = if (q.isBlank()) items else items.filter { matcher(q, it) }
val state = drag
if (state != null && base.none { it.key == state.key }) drag = null
val rows = drag?.let { activeListGapRows(base, it.key, it.index, it.height) } ?: base
// Rebuilding the model fires a list-wide repaint, so skip it when the visible rows are
// structurally unchanged (e.g. a stats/name refresh that produced identical rows) and only
// reconcile selection below. Row types are data classes, so equality is by value.
if (model.items != rows) {
setHovered(-1)
model.replaceAll(rows)
restoring = true
try {
// Rebuilding the model fires a list-wide repaint, so skip it when the visible rows are
// structurally unchanged (e.g. a stats/name refresh that produced identical rows) and only
// reconcile selection below. Row types are data classes, so equality is by value.
if (model.items != rows) {
setHovered(-1)
model.replaceAll(rows)
}
syncCellHeight(rows)
restore(rows, absent, scroll, at, reanchor)
} finally {
restoring = false
}
syncCellHeight(rows)
val idx = at?.let { activeListIndex(rows, it) }?.takeIf { it >= 0 }
?: activeListIndex(rows, prefer).takeIf { it >= 0 }
?: rows.indices.firstOrNull()
?: -1
if (idx >= 0) choose(idx, scroll) else list.clearSelection()
}
@RequiresEdt
private fun restore(rows: List<ActiveListItem>, absent: Absent, scroll: Boolean, at: Int?, reanchor: Boolean) {
checkEdt()
val idx = at?.let { activeListIndex(rows, it) }?.takeIf { it >= 0 }
if (idx != null) {
choose(idx, scroll)
mark = idx
if (reanchor) anchor = setOf(rows[idx].identity)
return
}
val indices = anchor.mapNotNull { id -> activeListIdentityIndex(rows, id).takeIf { it >= 0 } }
if (indices.isNotEmpty()) {
list.selectedIndices = indices.toIntArray()
mark = indices.first()
if (scroll) ScrollingUtil.ensureIndexIsVisible(list, mark, 0)
return
}
when (absent) {
Absent.KEEP -> {
list.clearSelection()
}
Absent.SLIDE -> {
if (anchor.isEmpty() || rows.isEmpty()) {
list.clearSelection()
mark = -1
return
}
val next = mark.coerceIn(0, rows.lastIndex)
choose(next, scroll)
anchor = setOf(rows[next].identity)
mark = next
}
Absent.FIRST -> {
if (rows.isEmpty()) {
list.clearSelection()
return
}
choose(0, scroll)
mark = 0
if (anchor.isEmpty()) anchor = setOf(rows[0].identity)
}
}
}
private fun identities(): Set<Any> = list.selectedValuesList.map { it.identity }.toSet()
@RequiresEdt
private fun setHovered(idx: Int) {
checkEdt()
@@ -436,12 +505,12 @@ internal class ActiveListView(
@RequiresEdt
fun primary() {
checkEdt()
val item = list.selectedValue ?: return
val item = active() ?: return
primary(item)
}
private fun open(focus: Boolean) {
val item = list.selectedValue ?: return
val item = active() ?: return
if (item is ActiveListGap) return
if (item.deleting) return
val action = onOpen
@@ -459,6 +528,13 @@ internal class ActiveListView(
onOpen?.invoke(item, true)
}
private fun active(): ActiveListItem? {
list.selectedValue?.let { return it }
if (model.size == 0) return null
choose(0, scroll = false)
return list.selectedValue
}
/**
* Default action for a double-click. Resolves to the row's explicit activation only: an
* [onActivate] handler, then the row's [ActiveListItem.doubleClick] cell, then a [primary] cell.
@@ -575,7 +651,7 @@ internal class ActiveListView(
val next = idx.coerceIn(run.first, run.last)
if (state.index == next && current != null) return
drag = state.copy(index = next)
sync(key, next, scroll = false)
sync(Absent.KEEP, scroll = false, at = next)
}
@RequiresEdt
@@ -594,7 +670,9 @@ internal class ActiveListView(
rows.add(target, item)
items = rows
heightKey = null
sync(state.key, target, scroll = false)
anchor = setOf(state.key)
mark = target
sync(Absent.KEEP, scroll = false, at = target)
if (source == target) return
reorder?.onMove?.invoke(ActiveListMove(state.key, source, target, rows.map { it.key }))
}
@@ -757,6 +835,8 @@ internal class ActiveListView(
val rows: List<ActiveListHeightRow>,
)
private enum class Absent { KEEP, SLIDE, FIRST }
}
private data class ActiveListHeightRow(
@@ -790,18 +870,25 @@ private fun activeListHeightRow(item: ActiveListItem): ActiveListHeightRow {
}
private fun activeListIndex(items: List<ActiveListItem>, key: String?): Int {
if (key == null) return if (items.isEmpty()) -1 else 0
if (key == null) return -1
return items.indexOfFirst { it.key == key }
}
private fun activeListIdentityIndex(items: List<ActiveListItem>, id: Any): Int {
return items.indexOfFirst { it.identity == id || it.key == id }
}
private fun activeListIndex(items: List<ActiveListItem>, index: Int): Int {
if (items.isEmpty()) return -1
return index.coerceIn(0, items.lastIndex)
}
internal sealed interface ActiveListSelection {
/** Restore the remembered rows and scroll to them; select nothing while they are absent. */
data object Preserve : ActiveListSelection
data object PreserveNoScroll : ActiveListSelection
/** Restore the remembered rows; when they are all gone, select the row that took their slot. */
data object Slide : ActiveListSelection
data class Key(val key: String) : ActiveListSelection
data class Index(val index: Int) : ActiveListSelection
}
@@ -234,6 +234,24 @@ class AgentManagerPanelTest : BasePlatformTestCase() {
assertEquals(second.id, edt { (list.selectedValue as ActiveListItem).key })
}
fun `test panel refresh keeps selected worktree across tab switch reload`() {
val first = WorktreeDto("/repo/.kilo/worktrees/feature-x", "feature-x", "feature/x", "/repo/.kilo/worktrees/feature-x")
val second = WorktreeDto("/repo/.kilo/worktrees/feature-y", "feature-y", "feature/y", "/repo/.kilo/worktrees/feature-y")
rpc.listed += first
rpc.listed += second
val controller = WorktreeController(service, "/test", coroutines.scope)
val panel = edt { AgentManagerPanel(testRootDisposable, controller, project) }
edt { controller.reload() }
flush()
val list = edt { UIUtil.findComponentOfType(panel, JBList::class.java)!! }
edt { list.selectedIndex = 1 }
edt { panel.refresh() }
flush()
assertEquals(second.id, edt { (list.selectedValue as ActiveListItem).key })
}
fun `test refresh keeps existing selection`() {
val first = WorktreeDto("/repo/.kilo/worktrees/feature-x", "feature-x", "feature/x", "/repo/.kilo/worktrees/feature-x")
val second = WorktreeDto("/repo/.kilo/worktrees/feature-y", "feature-y", "feature/y", "/repo/.kilo/worktrees/feature-y")
@@ -636,6 +654,49 @@ class AgentManagerPanelTest : BasePlatformTestCase() {
assertEquals(listOf(listOf(b.path, a.path)), rpc.reorders.toList())
}
fun `test dragging a worktree keeps dropped row selected after reorder reload`() {
val a = worktree("aardvark")
val b = worktree("beluga")
rpc.listed += main()
rpc.listed += a
rpc.listed += b
val controller = WorktreeController(service, project.basePath!!, coroutines.scope)
val panel = edt { AgentManagerPanel(testRootDisposable, controller, project) }
edt { controller.reload() }
flush()
val view = edt { UIUtil.findComponentOfType(panel, ActiveListView::class.java)!! }
layout(view)
edt {
assertTrue(view.select(b.id))
view.over(b.id, rowCenter(view, 1))
view.drop()
}
flush()
assertEquals(b.id, edt { view.selected()?.key })
}
fun `test renaming selected worktree keeps it selected`() {
val item = worktree("aardvark")
rpc.listed += main()
rpc.listed += item
val controller = WorktreeController(service, project.basePath!!, coroutines.scope)
val panel = edt { AgentManagerPanel(testRootDisposable, controller, project) }
edt { controller.reload() }
flush()
val view = edt { UIUtil.findComponentOfType(panel, ActiveListView::class.java)!! }
edt {
assertTrue(view.select(item.id))
controller.rename(item, "renamed", onFailure = {})
}
flush()
assertEquals(item.id, edt { view.selected()?.key })
assertEquals("renamed", edt { (view.selected() as ActiveListItem).title })
}
fun `test the current and pending rows are not draggable`() {
rpc.listed += main()
rpc.listed += worktree("aardvark")
@@ -52,6 +52,20 @@ class ActiveListReorderTest : BasePlatformTestCase() {
assertEquals(listOf("cur", "c", "a", "b"), move.keys)
}
fun `test drop anchors the moved row for owner refresh`() {
val moves = mutableListOf<ActiveListMove>()
val view = view(moves)
view.update(sectioned())
layout(view)
assertTrue(view.select("c"))
view.over("c", center(view, 1))
view.drop()
view.update(sectioned().let { listOf(it[0], it[3], it[1], it[2]) })
assertEquals("c", view.selected()?.key)
}
fun `test drag cannot leave its section and never displaces the current row`() {
val moves = mutableListOf<ActiveListMove>()
val view = view(moves)
@@ -0,0 +1,135 @@
package ai.kilocode.client.ui.list
import com.intellij.testFramework.fixtures.BasePlatformTestCase
class ActiveListSelectionTest : BasePlatformTestCase() {
fun `test two phase rebuild keeps selection and mutes onSelect`() {
var calls = 0
val view = view { calls++ }
view.update(rows("a", "b", "c"))
calls = 0
assertTrue(view.select("b"))
assertEquals(1, calls)
calls = 0
view.update(rows("a", "c"))
view.update(rows("a", "b", "c"))
assertEquals("b", view.selected()?.key)
assertEquals(0, calls)
}
fun `test preserve keeps absent anchor pending`() {
val view = view()
view.update(rows("a", "b"))
assertTrue(view.select("b"))
view.update(rows("a"), ActiveListSelection.Preserve)
assertNull(view.selected())
view.update(rows("a", "b"), ActiveListSelection.Preserve)
assertEquals("b", view.selected()?.key)
}
fun `test stable key preserves selection when value changes`() {
val view = view()
view.update(listOf(row("a", "Alpha"), row("b", "Beta")))
assertTrue(view.select("b"))
view.update(listOf(row("a", "Alpha"), row("b", "Beta changed")))
assertEquals("b", view.selected()?.key)
assertEquals("Beta changed", view.selected()?.title)
}
fun `test slide selects row that took selected slot`() {
val view = view()
view.update(rows("a", "b", "c"))
assertTrue(view.select("b"))
view.update(rows("a", "c"), ActiveListSelection.Slide)
assertEquals("c", view.selected()?.key)
view.update(emptyList(), ActiveListSelection.Slide)
assertNull(view.selected())
}
fun `test slide clears when nothing was selected`() {
val view = view()
view.update(rows("a", "b"))
view.clearSelection()
view.update(rows("a"), ActiveListSelection.Slide)
assertNull(view.selected())
}
fun `test filter selects first match and clearing restores anchor`() {
val view = view()
view.update(rows("a", "b", "c"))
assertTrue(view.select("b"))
view.filter("Alpha")
assertEquals("a", view.selected()?.key)
view.filter("")
assertEquals("b", view.selected()?.key)
}
fun `test multi select restores all surviving rows`() {
val view = ActiveListView("", ActiveListConfig(selection = javax.swing.ListSelectionModel.MULTIPLE_INTERVAL_SELECTION)) { _, _ -> }
view.update(rows("a", "b", "c", "d"))
view.setSelectionIndices(intArrayOf(1, 3))
view.update(rows("a", "b", "c", "d"))
assertEquals(listOf("b", "d"), view.selectedKeys())
view.update(rows("a", "b", "c"))
assertEquals(listOf("b"), view.selectedKeys())
}
fun `test absent select creates pending anchor`() {
val view = view()
view.update(rows("a"))
assertFalse(view.select("b"))
assertNull(view.selected())
view.update(rows("a", "b"))
assertEquals("b", view.selected()?.key)
}
fun `test identity override restores by identity and key`() {
val view = view()
view.update(listOf(row("pending", "Pending", "same")))
assertTrue(view.select("pending"))
view.update(listOf(row("created", "Created", "same")))
assertEquals("created", view.selected()?.key)
assertTrue(view.select("created"))
view.update(listOf(row("created", "Created again", "other")))
assertEquals("created", view.selected()?.key)
}
private fun view(onSelect: () -> Unit = {}): ActiveListView {
return ActiveListView("") { _, _ -> }.apply { this.onSelect = onSelect }
}
private fun rows(vararg keys: String): List<ActiveListItem> = keys.map { row(it, title(it)) }
private fun row(key: String, title: String, identity: Any = key): ActiveListItem = object : ActiveListItem {
override val key = key
override val identity = identity
override val title = title
override val search = title
}
private fun title(key: String): String = when (key) {
"a" -> "Alpha"
"b" -> "Beta"
"c" -> "Gamma"
"d" -> "Delta"
else -> key
}
}