fix(jetbrains): show pending session deletion state

2026-09-21 05:52:35 +08:00 · 2026-07-28 09:14:21 -04:00
parent 3056986f23
commit 5b0acee016
13 changed files with 229 additions and 36 deletions
@@ -0,0 +1,5 @@
+---
+"@kilocode/kilo-jetbrains": patch
+---
+
+Show worktree sessions as deleting while removal is in progress and notify when deletion fails.
@@ -70,7 +70,9 @@ class KiloSessionRpcApiImpl internal constructor(
    override suspend fun create(directory: String): SessionDto {
        app.requireReady()
        log.info("create session: directory=$directory")
-        return workspaces.get(directory).createSession()
+        val session = workspaces.get(directory).createSession()
+        log.info("create session: id=${session.id}, directory=$directory")
+        return session
    }

    override suspend fun get(id: String, directory: String): SessionDto {
@@ -81,6 +83,7 @@ class KiloSessionRpcApiImpl internal constructor(

    override suspend fun delete(id: String, directory: String) {
        app.requireReady()
+        log.info("delete session: id=$id, directory=$directory")
        val dir = sessions.getDirectory(id, directory)
        workspaces.get(dir).deleteSession(id)
    }
@@ -1,19 +1,51 @@
 package ai.kilocode.backend.rpc

+import ai.kilocode.backend.app.KiloAppState
+import ai.kilocode.backend.app.KiloBackendAppService
+import ai.kilocode.backend.testing.FakeCliServer
+import ai.kilocode.backend.testing.MockCliServer
 import ai.kilocode.backend.testing.TestLog
 import ai.kilocode.rpc.dto.ChatEventDto
+import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.cancel
 import kotlinx.coroutines.cancelAndJoin
+import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.flow.toList
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
+import kotlin.test.AfterTest
 import kotlin.test.Test
 import kotlin.test.assertFailsWith
 import kotlin.test.assertTrue

 class KiloSessionRpcApiImplTest {
+    private val mock = MockCliServer()
+    private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
+    private val apps = mutableListOf<KiloBackendAppService>()
+
+    @AfterTest
+    fun tearDown() {
+        apps.forEach { it.dispose() }
+        apps.clear()
+        scope.cancel()
+        mock.close()
+    }
+
+    private fun app(log: TestLog): KiloBackendAppService {
+        return KiloBackendAppService.create(scope, FakeCliServer(mock), log).also { apps.add(it) }
+    }
+
+    private suspend fun ready(app: KiloBackendAppService) {
+        app.connect()
+        withTimeout(10_000) {
+            app.appState.first { it is KiloAppState.Ready }
+        }
+    }

    @Test
    fun `events logs normal completion`() = runBlocking(Dispatchers.Default) {
@@ -49,4 +81,29 @@ class KiloSessionRpcApiImplTest {

        assertTrue(log.messages.any { it.contains("route=rpc-events stop=true failed message=stream failed") }, log.messages.joinToString("\n"))
    }
+
+    @Test
+    fun `create logs created session id`() = runBlocking(Dispatchers.Default) {
+        val log = TestLog()
+        mock.sessionCreate = """{"id":"ses_created","slug":"created","projectID":"prj_test","directory":"/test","title":"Created","version":"1.0.0","time":{"created":1000,"updated":1000}}"""
+        val app = app(log)
+        ready(app)
+        val api = KiloSessionRpcApiImpl(appOverride = app, log = log)
+
+        api.create("/test")
+
+        assertTrue(log.messages.any { it.contains("create session: id=ses_created") }, log.messages.joinToString("\n"))
+    }
+
+    @Test
+    fun `delete logs deleted session id`() = runBlocking(Dispatchers.Default) {
+        val log = TestLog()
+        val app = app(log)
+        ready(app)
+        val api = KiloSessionRpcApiImpl(appOverride = app, log = log)
+
+        api.delete("ses_deleted", "/test")
+
+        assertTrue(log.messages.any { it.contains("delete session: id=ses_deleted") }, log.messages.joinToString("\n"))
+    }
 }
@@ -1,5 +1,6 @@
 package ai.kilocode.client.agentManager.worktree

+import ai.kilocode.client.KiloNotifications
 import ai.kilocode.client.app.KiloSessionService
 import ai.kilocode.client.app.KiloWorkspaceService
 import ai.kilocode.client.app.Workspace
@@ -46,8 +47,10 @@ open class WorktreeSessionEditorManager(
    private val confirm: (JComponent, String, String) -> Boolean = { parent, msg, title ->
        Messages.showYesNoDialog(parent, msg, title, Messages.getWarningIcon()) == Messages.YES
    },
+    private val notify: (String, String?) -> Unit = { title, content -> KiloNotifications.error(project, title, content) },
 ) : SessionHost(project, worktree, create, resolve, status, timers, request) {
    private val right = JPanel(BorderLayout())
+    private val deleting = linkedSetOf<String>()
    private var last: String? = null
    private var pending = false
    var onPresent: ((String?) -> Unit)? = null
@@ -71,6 +74,9 @@ open class WorktreeSessionEditorManager(
    @RequiresEdt
    open fun hasPendingNew(): Boolean = pending

+    @RequiresEdt
+    open fun deleting(): Set<String> = deleting
+
    @RequiresEdt
    override fun newSession() {
        newSession(focus = true)
@@ -111,7 +117,7 @@ open class WorktreeSessionEditorManager(

    @RequiresEdt
    open fun deleteSessions(ids: List<String>) {
-        val active = ids.filter { it != NEW }.distinct()
+        val active = ids.filter { it != NEW && it !in deleting }.distinct()
        if (active.isEmpty()) return
        val msg = if (active.size == 1)
            KiloBundle.message("worktree.session.delete.confirm.message", title(active[0]))
@@ -119,16 +125,22 @@ open class WorktreeSessionEditorManager(
            KiloBundle.message("worktree.session.delete.confirm.message.multiple", active.size)
        if (!confirm(right, msg, KiloBundle.message("worktree.session.delete.confirm.title"))) return
        val key = currentKey()
-        val show = key in active
-        active.forEach(::forceSession)
-        list.delete(active) {
-            if (!show) {
+        val names = active.associateWith(::title)
+        deleting.addAll(active)
+        onListChanged?.invoke()
+        active.forEach { id ->
+            val name = names[id] ?: title(id)
+            list.delete(id) { ok, err ->
+                deleting.remove(id)
                onListChanged?.invoke()
-                return@delete
+                if (ok) return@delete
+                notify(KiloBundle.message("worktree.session.delete.failed.title", name), err)
            }
+        }
+        active.forEach(::forceSession)
+        if (key in active) {
            val next = latest()
            if (next != null) openSession(SessionRef.Local(next)) else newSession()
-            onListChanged?.invoke()
        }
    }

@@ -151,6 +163,7 @@ open class WorktreeSessionEditorManager(
    private fun latest(): SessionDto? {
        return (0 until list.model.size)
            .map { list.model.getElementAt(it) }
+            .filter { it.id !in deleting }
            .maxByOrNull { it.time.updated }
    }

@@ -124,6 +124,7 @@ class WorktreeSessionEditorPanel(

    @RequiresEdt
    private fun open(row: ActiveListItem, focus: Boolean) {
+        if (row.key in manager.deleting()) return
        if (row.key == SessionHost.NEW) {
            manager.newSession()
            return
@@ -138,9 +139,10 @@ class WorktreeSessionEditorPanel(
        val key = manager.currentKey()
        val pending = manager.hasPendingNew()
        val kinds = manager.activity()
+        val deleting = manager.deleting()
        if (pending || key == SessionHost.NEW) rows += NewRow
        rows += HistoryTime.sorted((0 until controller.model.size).map { LocalHistoryItem(controller.model.getElementAt(it)) })
-            .map { SessionRow(it.session, kinds[it.id]) }
+            .map { SessionRow(it.session, kinds[it.id], deleting = it.id in deleting) }
        list.update(rows, ActiveListSelection.PreserveNoScroll)
        select(if (pending) SessionHost.NEW else key)
        updateActions()
@@ -160,7 +162,7 @@ class WorktreeSessionEditorPanel(
    }

    @RequiresEdt
-    private fun selectedKeys(): List<String> = list.selectedKeys().filter { it != SessionHost.NEW }
+    private fun selectedKeys(): List<String> = list.selectedKeys().filter { it != SessionHost.NEW && it !in manager.deleting() }

    @RequiresEdt
    private fun updateActions() {
@@ -230,18 +232,23 @@ class WorktreeSessionEditorPanel(
        override val section: String get() = HistoryTime.title(HistorySection.TODAY)
    }

-    private data class SessionRow(val session: SessionDto, val kind: SessionActivityKind?) : ActiveListItem {
+    private data class SessionRow(
+        val session: SessionDto,
+        val kind: SessionActivityKind?,
+        val deleting: Boolean = false,
+    ) : ActiveListItem {
        private val item = LocalHistoryItem(session)
        override val key: String get() = session.id
        override val title: String get() = session.title.takeIf { it.isNotBlank() }
            ?: KiloBundle.message("worktree.session.untitled")
        override val tooltip: String get() = title
-        override val badges: List<ActiveListBadge> get() = listOfNotNull(kind?.let { ActiveListBadge(it.label(), it.style()) })
-        override val trailing: String get() = HistoryTime.relative(item)
+        override val badges: List<ActiveListBadge> get() = if (deleting) emptyList() else listOfNotNull(kind?.let { ActiveListBadge(it.label(), it.style()) })
+        override val trailing: String get() = if (deleting) KiloBundle.message("worktree.session.deleting") else HistoryTime.relative(item)
        override val section: String get() = HistoryTime.title(HistoryTime.section(item))
        override val search: String get() = listOf(session.title, session.id, session.directory).joinToString(" ")
+        override val muted: Boolean get() = deleting
        override val cells: List<ActiveListCell>
-            get() = listOf(ActiveListCell(
+            get() = if (deleting) emptyList() else listOf(ActiveListCell(
                DELETE_CELL,
                KiloBundle.message("worktree.session.delete.action"),
                icon = AllIcons.Actions.GC,
@@ -52,29 +52,24 @@ class WorktreeSessionListController(
        }
    }

-    fun delete(ids: List<String>, done: () -> Unit) {
-        val active = ids.distinct().filter { it.isNotBlank() }
-        if (active.isEmpty()) {
-            edt(done)
-            return
-        }
+    fun delete(id: String, done: (Boolean, String?) -> Unit) {
+        if (id.isBlank()) return edt { done(false, "Missing session id") }
        cs.launch {
-            try {
-                active.forEach { id ->
-                    service.deleteSession(id, dir)
-                    capture("Worktree Session Deleted", mapOf("sessionId" to id))
-                }
+            val result = runCatching { service.deleteSession(id, dir) }
+            if (result.isSuccess) {
                edt {
                    val keep = (0 until model.size)
                        .map { model.getElementAt(it) }
-                        .filter { it.id !in active }
+                        .filter { it.id != id }
                    model.replaceAll(keep)
-                    done()
+                    capture("Worktree Session Deleted", mapOf("sessionId" to id))
+                    done(true, null)
                }
-            } catch (e: Exception) {
-                LOG.warn("worktree session delete failed dir=$dir message=${e.message}", e)
-                edt { done() }
+                return@launch
            }
+            val err = result.exceptionOrNull()
+            LOG.warn("worktree session delete failed id=$id dir=$dir message=${err?.message}", err)
+            edt { done(false, err?.message) }
            reload()
        }
    }
@@ -112,9 +112,9 @@ class KiloSessionService internal constructor(

    /** Create a new session. Caller awaits the result. */
    suspend fun create(dir: String): SessionDto {
-        log.info("create: dir=$dir")
+        log.info("kind=session create=true dir=${ChatLogSummary.dir(dir)}")
        val session = call { create(dir) }
-        log.info("create: id=${session.id}")
+        log.info("${ChatLogSummary.sid(session.id)} kind=session create=true ok=true dir=${ChatLogSummary.dir(dir)}")
        refresh(dir)
        return session
    }
@@ -131,7 +131,9 @@ class KiloSessionService internal constructor(
    }

    suspend fun deleteSession(id: String, dir: String) {
+        log.info("${ChatLogSummary.sid(id)} kind=session delete=true dir=${ChatLogSummary.dir(dir)}")
        call { delete(id, dir) }
+        log.info("${ChatLogSummary.sid(id)} kind=session delete=true ok=true dir=${ChatLogSummary.dir(dir)}")
        list(dir)
    }

@@ -64,6 +64,7 @@ internal interface ActiveListItem {
    val trailing: String? get() = null
    val cells: List<ActiveListCell> get() = emptyList()
    val disabled: Boolean get() = false
+    val muted: Boolean get() = false
    /** Extra text matched by the filter field in addition to [title]; null matches title only. */
    val search: String? get() = null
 }
@@ -84,6 +84,7 @@ internal class ActiveListRenderer(
        val active = selected && (focused || list.hasFocus() || (list as? ActiveListActive)?.active() == true)
        val fg = UIUtil.getListForeground(active, active || focused)
        val weak = if (active) fg else UiStyle.Colors.weak()
+        val titleFg = if (value.muted) weak else fg
        val section = activeListSectionTitle(model.items, index)

        background = list.background
@@ -95,7 +96,7 @@ internal class ActiveListRenderer(
        top.setPreferredSize(section?.let { Dimension(0, sep.getFontMetrics(sep.font).height + insets.top + insets.bottom) })

        title.clear()
-        title.append(value.title, SimpleTextAttributes(SimpleTextAttributes.STYLE_BOLD, fg))
+        title.append(value.title, SimpleTextAttributes(SimpleTextAttributes.STYLE_BOLD, titleFg))
        value.note?.takeIf { it.isNotBlank() }?.let {
            title.append("  $it", SimpleTextAttributes.GRAYED_ATTRIBUTES)
        }
@@ -322,6 +322,8 @@ worktree.session.delete.action=Delete session
 worktree.session.delete.confirm.title=Delete session?
 worktree.session.delete.confirm.message=Delete session "{0}"?
 worktree.session.delete.confirm.message.multiple=Delete {0} sessions?
+worktree.session.delete.failed.title=Failed to delete session "{0}"
+worktree.session.deleting=Deleting…
 worktree.session.new=New session
 worktree.session.untitled=Untitled session
 worktree.menu.from=New Worktree from {0}
@@ -26,6 +26,7 @@ import com.intellij.openapi.ui.TestDialogManager
 import com.intellij.openapi.util.Disposer
 import com.intellij.testFramework.fixtures.BasePlatformTestCase
 import com.intellij.util.ui.UIUtil
+import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.flow.MutableStateFlow
 import javax.swing.JComponent
 import javax.swing.JPanel
@@ -41,6 +42,7 @@ class WorktreeSessionEditorManagerTest : BasePlatformTestCase() {
    private lateinit var timers: TestUiTimers
    private val created = mutableListOf<Pair<String, String?>>()
    private val requested = mutableListOf<JComponent>()
+    private val notified = mutableListOf<Pair<String, String?>>()
    private val ui = mutableListOf<SessionUi>()
    private var confirms = 0

@@ -146,15 +148,52 @@ class WorktreeSessionEditorManagerTest : BasePlatformTestCase() {
        rpc.listed += first
        rpc.listed += second
        val manager = manager()
-        edt { manager.openSession(SessionRef.Local(first)) }
-        val removed = ui.single()
+        edt { manager.start() }
+        flush()

        edt { manager.deleteSessions(listOf(first.id)) }
        pump()
        flush()

        assertEquals(1, confirms)
-        assertTrue(confirms > 0)
+        assertEquals(listOf(DIR to "ses_1", DIR to "ses_2"), created)
+        waitUntil { manager.deleting().isEmpty() }
+        assertEquals(listOf(first.id to DIR), rpc.deletes.toList())
+    }
+
+    fun `test delete marks session deleting then removes on success`() {
+        val gate = CompletableDeferred<Unit>()
+        rpc.deleteGate = gate
+        val session = session("ses_1", updated = 1.0)
+        val manager = manager()
+
+        edt { manager.deleteSessions(listOf(session.id)) }
+        pump()
+
+        assertEquals(setOf(session.id), edt { manager.deleting() })
+
+        gate.complete(Unit)
+        waitUntil { manager.deleting().isEmpty() && rpc.deletes.contains(session.id to DIR) }
+
+        assertTrue(edt { manager.deleting().isEmpty() })
+        assertEquals(listOf(session.id to DIR), rpc.deletes.toList())
+        assertTrue(notified.isEmpty())
+    }
+
+    fun `test delete failure reverts row and notifies`() {
+        val session = session("ses_1", updated = 1.0)
+        rpc.listed += session
+        rpc.deleteThrows = IllegalStateException("delete unavailable")
+        val manager = manager()
+        edt { manager.start() }
+        flush()
+
+        edt { manager.deleteSessions(listOf(session.id)) }
+        waitUntil { manager.deleting().isEmpty() }
+
+        assertTrue(edt { manager.deleting().isEmpty() })
+        assertTrue(rpc.listed.any { it.id == session.id })
+        assertEquals(listOf("Failed to delete session \"Session ses_1\"" to "delete unavailable"), notified)
    }

    private fun manager(focus: Boolean = false): WorktreeSessionEditorManager {
@@ -192,6 +231,7 @@ class WorktreeSessionEditorManagerTest : BasePlatformTestCase() {
            timers = timers,
            request = { requested += it },
            confirm = { _, _, _ -> confirms++; true },
+            notify = { title, content -> notified += title to content },
        ).also { it.startFocus = focus }
    }

@@ -206,6 +246,14 @@ class WorktreeSessionEditorManagerTest : BasePlatformTestCase() {

    private fun flush() = coroutines.drain(::pump)

+    private fun waitUntil(block: () -> Boolean) {
+        repeat(10) {
+            flush()
+            if (edt(block)) return
+        }
+        assertTrue(edt(block))
+    }
+
    private fun pump() {
        com.intellij.openapi.application.ApplicationManager.getApplication().invokeAndWait {
            UIUtil.dispatchAllInvocationEvents()
@@ -123,6 +123,21 @@ class WorktreeSessionEditorPanelTest : BasePlatformTestCase() {
        assertEquals(HistoryTime.title(HistoryTime.section(LocalHistoryItem(session))), row.section)
    }

+    fun `test deleting row shows deleting state`() {
+        manager.kinds = mapOf("ses_1" to SessionActivityKind.RUNNING)
+        manager.deletingIds += "ses_1"
+        rpc.listed += session("ses_1", nowSeconds())
+        edt { controller.reload() }
+        flush()
+
+        val row = row("ses_1")
+
+        assertEquals("Deleting…", row.trailing)
+        assertTrue(row.cells.isEmpty())
+        assertTrue(row.badges.isEmpty())
+        assertTrue(row.muted)
+    }
+
    fun `test pending new session groups under today`() {
        manager.pending = true
        rpc.listed += session("ses_today", nowSeconds())
@@ -167,6 +182,24 @@ class WorktreeSessionEditorPanelTest : BasePlatformTestCase() {
        assertEquals(listOf(false), manager.focuses)
    }

+    fun `test row click ignores deleting session`() {
+        manager.deletingIds += "ses_1"
+        rpc.listed += session("ses_1", 1.0)
+        edt { controller.reload() }
+        flush()
+
+        val list = edt { UIUtil.findComponentOfType(panel, JBList::class.java)!! }
+        edt {
+            list.setSize(400, 100)
+            list.doLayout()
+            val bounds = list.getCellBounds(0, 0)
+            fire(list, MouseEvent(list, MouseEvent.MOUSE_CLICKED, System.currentTimeMillis(), 0, bounds.x + 8, bounds.y + bounds.height / 2, 1, false, MouseEvent.BUTTON1))
+        }
+
+        assertTrue(manager.refs.isEmpty())
+        assertTrue(manager.focuses.isEmpty())
+    }
+
    fun `test row double click opens and focuses session`() {
        rpc.listed += session("ses_1", 1.0)
        edt { controller.reload() }
@@ -217,6 +250,27 @@ class WorktreeSessionEditorPanelTest : BasePlatformTestCase() {
        assertEquals(listOf("ses_2", "ses_1"), manager.deleted)
    }

+    fun `test delete action skips deleting selected sessions`() {
+        manager.deletingIds += "ses_1"
+        rpc.listed += session("ses_1", 1.0)
+        rpc.listed += session("ses_2", 2.0)
+        edt { controller.reload() }
+        flush()
+
+        edt {
+            panel.selectSessions(listOf("ses_1"))
+            panel.deleteSelected()
+        }
+        assertTrue(manager.deleted.isEmpty())
+
+        edt {
+            panel.selectSessions(listOf("ses_1", "ses_2"))
+            panel.deleteSelected()
+        }
+
+        assertEquals(listOf("ses_2"), manager.deleted)
+    }
+
    fun `test panel provides session manager and workspace data`() {
        val sink = SessionSink()

@@ -292,6 +346,7 @@ class WorktreeSessionEditorPanelTest : BasePlatformTestCase() {
        var newCount = 0
        var pending = false
        var kinds = emptyMap<String, SessionActivityKind>()
+        val deletingIds = mutableSetOf<String>()
        val refs = mutableListOf<String>()
        val focuses = mutableListOf<Boolean>()
        val deleted = mutableListOf<String>()
@@ -300,6 +355,8 @@ class WorktreeSessionEditorPanelTest : BasePlatformTestCase() {

        override fun activity(): Map<String, SessionActivityKind> = kinds

+        override fun deleting(): Set<String> = deletingIds
+
        override fun newSession() {
            newCount++
        }
@@ -104,6 +104,7 @@ class FakeSessionRpcApi : KiloSessionRpcApi {
    val questionRejects = mutableListOf<Pair<String, String>>()
    val deletes = java.util.concurrent.CopyOnWriteArrayList<Pair<String, String>>()
    var deleteGate: CompletableDeferred<Unit>? = null
+    var deleteThrows: Exception? = null
    val renames = mutableListOf<Triple<String, String, String>>()
    var renameThrows: Exception? = null
    val lists = mutableListOf<String>()
@@ -150,6 +151,7 @@ class FakeSessionRpcApi : KiloSessionRpcApi {

    override suspend fun delete(id: String, directory: String) {
        assertNotEdt("delete")
+        deleteThrows?.let { throw it }
        deleteGate?.await()
        deletes.add(id to directory)
        listed.removeAll { it.id == id }