From 2127b8b4ebb379ab5734dcf989e4817423655f13 Mon Sep 17 00:00:00 2001 From: kirillk Date: Tue, 25 Aug 2026 11:46:29 -0400 Subject: [PATCH 1/2] fix(jetbrains): harden agent manager worktrees --- .changeset/jetbrains-worktree-safety.md | 5 + .../backend/rpc/KiloWorkspaceRpcApiImpl.kt | 4 + .../backend/rpc/KiloWorktreeRpcApiImpl.kt | 82 +++++++-- .../backend/workspace/KiloBackendWorkspace.kt | 7 + .../backend/workspace/KiloWorkspaceState.kt | 1 + .../backend/rpc/KiloWorktreeRpcApiImplTest.kt | 174 ++++++++++++++++++ .../workspace/KiloBackendWorkspaceTest.kt | 69 +++++-- .../session/controller/SessionController.kt | 8 + .../resources/messages/KiloBundle.properties | 2 + .../agentManager/WorktreeControllerTest.kt | 19 ++ .../session/controller/ConnectionDelayTest.kt | 24 +++ .../kilocode/rpc/dto/KiloWorkspaceStateDto.kt | 1 + .../kotlin/ai/kilocode/rpc/dto/WorktreeDto.kt | 1 + 13 files changed, 369 insertions(+), 28 deletions(-) create mode 100644 .changeset/jetbrains-worktree-safety.md diff --git a/.changeset/jetbrains-worktree-safety.md b/.changeset/jetbrains-worktree-safety.md new file mode 100644 index 0000000000..40e619f1b0 --- /dev/null +++ b/.changeset/jetbrains-worktree-safety.md @@ -0,0 +1,5 @@ +--- +"@kilocode/kilo-jetbrains": patch +--- + +Keep JetBrains Agent Manager worktrees in the main repository storage, prevent nested worktree deletion from removing child worktrees, and show a clear missing-folder error for deleted workspaces. diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorkspaceRpcApiImpl.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorkspaceRpcApiImpl.kt index 4abe1975d6..15bfc06642 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorkspaceRpcApiImpl.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorkspaceRpcApiImpl.kt @@ -508,6 +508,10 @@ class KiloWorkspaceRpcApiImpl internal constructor( status = KiloWorkspaceStatusDto.UNSUPPORTED, error = state.reason, ) + is KiloWorkspaceState.Missing -> KiloWorkspaceStateDto( + status = KiloWorkspaceStatusDto.MISSING, + error = state.path, + ) is KiloWorkspaceState.Error -> KiloWorkspaceStateDto( status = KiloWorkspaceStatusDto.ERROR, error = state.message, diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt index 44bd19f386..dc5d0b9e1f 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt @@ -83,9 +83,10 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { val res = runGit(base, "worktree", "list", "--porcelain") if (!res.ok) return@withContext WorktreeListDto() val items = managedWorktrees(parseWorktreeList(res.stdout)) - val store = worktreeNameStore(items) - val state = store?.let { syncWorktreeState(it, worktreePaths(items)) } ?: WorktreeState() - val named = overlayWorktreeNames(items, state.names) + val alive = items.filter { it.main || Files.isDirectory(Path.of(it.path)) } + val store = worktreeNameStore(alive) + val state = store?.let { syncWorktreeState(it, worktreePaths(alive)) } ?: WorktreeState() + val named = overlayWorktreeNames(alive, state.names) WorktreeListDto(orderWorktrees(named, state.worktreeOrder)) } @@ -276,6 +277,14 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { return Path.of(lines[0]).normalize() != Path.of(lines[1]).normalize() } + /** Main working tree for the repo containing [base]; falls back to [base] when git fails. */ + private fun mainWorktree(base: Path): Path { + val res = runGit(base, "worktree", "list", "--porcelain") + if (!res.ok) return base + val main = parseWorktreeList(res.stdout).firstOrNull { it.main } ?: return base + return Path.of(main.path).normalize() + } + override suspend fun create(directory: String, request: CreateWorktreeRequestDto): CreateWorktreeResultDto = withContext(Dispatchers.IO) { val base = Path.of(directory).normalize() @@ -312,7 +321,12 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { /** Runs `git worktree add` under `/.kilo/worktrees/` and records list bookkeeping. */ private fun addWorktree(base: Path, branch: String, existing: Boolean, baseRef: String?): CreateWorktreeResultDto { - val dir = base.resolve(".kilo").resolve("worktrees").resolve(branch.replace('/', '-')) + val root = mainWorktree(base) + val storage = root.resolve(".kilo").resolve("worktrees").normalize() + val parts = branch.split('/') + if (parts.any { it.isBlank() || it == "." || it == ".." }) return CreateWorktreeResultDto(error = "Invalid branch name") + val dir = storage.resolve(branch.replace('/', '-')).normalize() + if (dir.parent != storage) return CreateWorktreeResultDto(error = "Invalid branch name") Files.createDirectories(dir.parent) val args = buildList { addAll(listOf("worktree", "add")) @@ -327,7 +341,7 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { } } LOG.info("worktree add requested: branch=$branch existing=$existing base=${baseRef ?: "(current)"} dir=$dir") - val res = runGit(base, *args.toTypedArray()) + val res = add(base, args) if (!res.ok) { LOG.warn("worktree add failed: branch=$branch exit=${res.exit} stderr=${res.stderr.trim()}") return CreateWorktreeResultDto(error = res.stderr.ifBlank { "git worktree add failed" }) @@ -347,15 +361,37 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { val base = Path.of(directory).normalize() LOG.info("worktree remove requested: path=$path branch=${branch ?: "(none)"} force=$force base=$base") val list = runGit(base, "worktree", "list", "--porcelain") - val store = (if (list.ok) worktreeNameStore(managedWorktrees(parseWorktreeList(list.stdout))) else null) - ?: base.resolve(".kilo").resolve(WORKTREE_NAMES_FILE) + if (!list.ok) return@withContext RemoveWorktreeResultDto(error = list.stderr.ifBlank { "git worktree list failed" }) + val all = parseWorktreeList(list.stdout) + val items = managedWorktrees(all) + val main = all.firstOrNull { it.main } + val storage = main?.let { Path.of(it.path).normalize().resolve(".kilo").resolve("worktrees").normalize() } + val target = all.firstOrNull { + val item = Path.of(it.path).normalize() + !it.main && samePath(it.path, path) && item.parent == storage + } + ?: return@withContext RemoveWorktreeResultDto(error = "Refusing to remove unmanaged worktree: $path") + val root = Path.of(path).normalize() + val nested = all.filter { + val item = Path.of(it.path).normalize() + !it.prunable && Files.isDirectory(item) && !samePath(it.path, path) && item.startsWith(root) + } + if (nested.isNotEmpty()) { + val names = nested.joinToString("\n") { it.path } + return@withContext RemoveWorktreeResultDto(error = "Delete nested worktrees first:\n$names") + } + val store = worktreeNameStore(items) ?: base.resolve(".kilo").resolve(WORKTREE_NAMES_FILE) // Force means the user accepted removing a locked worktree; unlock first so the plain // remove succeeds. Unlock fails harmlessly when the tree isn't actually locked. if (force) { - val unlock = runGit(base, "worktree", "unlock", path) + val unlock = runGit(base, "worktree", "unlock", target.path) if (!unlock.ok) LOG.info("worktree unlock skipped: path=$path exit=${unlock.exit} stderr=${unlock.stderr.trim()}") } - val res = runGit(base, "worktree", "remove", "--force", path) + val res = if (target.prunable || !Files.isDirectory(Path.of(target.path))) { + GitResult(0, "", "") + } else { + runGit(base, "worktree", "remove", "--force", target.path) + } if (!res.ok) { val locked = res.stderr.contains("locked working tree", ignoreCase = true) LOG.warn("worktree remove failed: path=$path locked=$locked exit=${res.exit} stderr=${res.stderr.trim()}") @@ -370,7 +406,11 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { if (!del.ok) LOG.warn("worktree branch delete failed: branch=$it exit=${del.exit} stderr=${del.stderr.trim()}") } LOG.info("worktree removed: path=$path branch=${branch ?: "(none)"}") - removeWorktreeState(store, path) + removeWorktreeState(store, target.path) + val prune = runGit(base, "worktree", "prune") + if (!prune.ok) LOG.warn("worktree prune failed: exit=${prune.exit} stderr=${prune.stderr.trim()}") + runCatching { service().workspaces.remove(target.path) } + .onFailure { err -> LOG.info("workspace cache eviction skipped: path=${target.path} message=${err.message}") } RemoveWorktreeResultDto(ok = true) } @@ -471,6 +511,20 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { } } + private fun add(base: Path, args: List): GitResult { + val first = runGit(base, *args.toTypedArray()) + if (first.ok || !stale(first.stderr)) return first + val prune = runGit(base, "worktree", "prune") + if (!prune.ok) LOG.warn("worktree prune before retry failed: exit=${prune.exit} stderr=${prune.stderr.trim()}") + return runGit(base, *args.toTypedArray()) + } + + private fun stale(text: String): Boolean { + return text.contains("is already checked out", ignoreCase = true) || + text.contains("already used by worktree", ignoreCase = true) || + text.contains("missing but already registered worktree", ignoreCase = true) + } + private suspend fun parallel(items: List, block: suspend (T) -> R): List = coroutineScope { val sem = Semaphore(4) items.map { item -> async { sem.withPermit { block(item) } } }.map { it.await() } @@ -637,16 +691,18 @@ internal fun parseWorktreeList(raw: String): List { var branch = "(detached)" var locked = false var lockReason: String? = null + var prunable = false var first = true fun flush() { val p = path ?: return val name = p.substringAfterLast('/').ifBlank { p } - out.add(WorktreeDto(p, name, branch, p, main = first, locked = locked, lockReason = lockReason)) + out.add(WorktreeDto(p, name, branch, p, main = first, locked = locked, lockReason = lockReason, prunable = prunable)) first = false path = null branch = "(detached)" locked = false lockReason = null + prunable = false } for (line in raw.lines()) { when { @@ -656,6 +712,7 @@ internal fun parseWorktreeList(raw: String): List { locked = true lockReason = line.removePrefix("locked").trim().takeIf { it.isNotEmpty() } } + line == "prunable" || line.startsWith("prunable ") -> prunable = true line.isBlank() -> flush() } } @@ -669,8 +726,9 @@ internal fun managedWorktrees(items: List): List { val storage = root.resolve(".kilo").resolve("worktrees").normalize() return items.filter { item -> if (item.main) return@filter true + if (item.prunable) return@filter false val path = Path.of(item.path).normalize() - path.startsWith(storage) && path != storage + path.parent == storage } } diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspace.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspace.kt index cd700fc209..c35d1e17a0 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspace.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspace.kt @@ -28,6 +28,8 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import okhttp3.OkHttpClient import okhttp3.Request +import java.nio.file.Files +import java.nio.file.Path import java.util.concurrent.atomic.AtomicReference /** @@ -77,6 +79,11 @@ class KiloBackendWorkspace( _state.value = KiloWorkspaceState.Unsupported(reason) return@launch } + if (!Files.isDirectory(Path.of(directory))) { + log.info("Workspace directory is missing: $directory") + _state.value = KiloWorkspaceState.Missing(directory) + return@launch + } val progress = AtomicReference(KiloWorkspaceLoadProgress()) _state.value = KiloWorkspaceState.Loading(progress.get()) diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloWorkspaceState.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloWorkspaceState.kt index c9a543b1ef..239ba178c8 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloWorkspaceState.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloWorkspaceState.kt @@ -20,6 +20,7 @@ sealed class KiloWorkspaceState { val skills: List, ) : KiloWorkspaceState() data class Unsupported(val reason: String) : KiloWorkspaceState() + data class Missing(val path: String) : KiloWorkspaceState() data class Error(val message: String, val errors: List = emptyList()) : KiloWorkspaceState() } diff --git a/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt b/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt index e19a9a386b..dcdb5991ec 100644 --- a/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt +++ b/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt @@ -80,6 +80,26 @@ class KiloWorktreeRpcApiImplTest { assertEquals("Air Agent worktree", list[1].lockReason) } + @Test + fun `parseWorktreeList captures the prunable flag`() { + val raw = """ + worktree /repo + HEAD 1111111111111111111111111111111111111111 + branch refs/heads/main + + worktree /repo/.kilo/worktrees/hyper-video + HEAD 2222222222222222222222222222222222222222 + branch refs/heads/hyper-video + prunable gitdir file points to non-existent location + + """.trimIndent() + + val list = parseWorktreeList(raw) + + assertFalse(list[0].prunable, "main tree is not prunable") + assertTrue(list[1].prunable, "second tree should be flagged prunable") + } + @Test fun `managedWorktrees keeps only agent manager worktrees`() { val raw = """ @@ -124,6 +144,33 @@ class KiloWorktreeRpcApiImplTest { assertEquals(listOf("/repo"), list.map { it.path }) } + @Test + fun `managedWorktrees rejects nested and prunable worktrees`() { + val raw = """ + worktree /repo + HEAD 1111111111111111111111111111111111111111 + branch refs/heads/main + + worktree /repo/.kilo/worktrees/feature-x + HEAD 2222222222222222222222222222222222222222 + branch refs/heads/feature/x + + worktree /repo/.kilo/worktrees/feature-x/.kilo/worktrees/nested + HEAD 3333333333333333333333333333333333333333 + branch refs/heads/nested + + worktree /repo/.kilo/worktrees/dead + HEAD 4444444444444444444444444444444444444444 + branch refs/heads/dead + prunable gitdir file points to non-existent location + + """.trimIndent() + + val list = managedWorktrees(parseWorktreeList(raw)) + + assertEquals(listOf("/repo", "/repo/.kilo/worktrees/feature-x"), list.map { it.path }) + } + @Test fun `classifyGhError detects missing and unauthorized gh states`() { assertEquals(GhAvailability.UNAUTH, classifyGhError("You are not logged into any GitHub hosts. Run gh auth login to authenticate.")) @@ -232,6 +279,44 @@ class KiloWorktreeRpcApiImplTest { assertFalse(after.any { it.branch == "feature/x" }, "removed worktree should be gone") } + @Test + fun `create from inside linked worktree uses main worktree storage`() = runBlocking { + initRepo() + val first = assertNotNull(api.create(repo.toString(), CreateWorktreeRequestDto("feature/x")).worktree) + + val result = api.create(first.path, CreateWorktreeRequestDto("feature/y")) + val created = assertNotNull(result.worktree, "create failed: ${result.error}") + + assertEquals(repo.resolve(".kilo").resolve("worktrees").resolve("feature-y").toRealPath().toString(), created.path) + assertFalse( + Files.exists(Path.of(first.path).resolve(".kilo").resolve("worktrees").resolve("feature-y")), + "creating from a linked worktree must not nest storage inside it", + ) + } + + @Test + fun `create rejects a branch slug that escapes storage`() = runBlocking { + initRepo() + + val result = api.create(repo.toString(), CreateWorktreeRequestDto("../escape")) + + assertNull(result.worktree) + assertEquals("Invalid branch name", result.error) + assertFalse(Files.exists(repo.resolve(".kilo").resolve("escape"))) + } + + @Test + fun `create succeeds after pruning a deleted checked out branch`() = runBlocking { + initRepo() + val first = assertNotNull(api.create(repo.toString(), CreateWorktreeRequestDto("feature/x")).worktree) + delete(Path.of(first.path)) + + val result = api.create(repo.toString(), CreateWorktreeRequestDto("feature/x", existingBranch = true)) + + val created = assertNotNull(result.worktree, "create should prune stale metadata and retry: ${result.error}") + assertTrue(Files.isDirectory(Path.of(created.path))) + } + @Test fun `create records order so reload keeps creation order`() = runBlocking { initRepo() @@ -359,6 +444,84 @@ class KiloWorktreeRpcApiImplTest { assertTrue(result.error != null, "failure should carry an error message") } + @Test + fun `remove refuses a path outside managed storage`() = runBlocking { + initRepo() + val outside = repo.resolve("outside") + Files.createDirectories(outside) + + val result = api.remove(repo.toString(), outside.toString(), null) + + assertFalse(result.ok) + assertTrue(result.error?.contains("Refusing") == true) + assertTrue(Files.isDirectory(outside), "unmanaged directory must not be touched") + } + + @Test + fun `remove refuses a worktree containing a live nested worktree`() = runBlocking { + initRepo() + val parent = assertNotNull(api.create(repo.toString(), CreateWorktreeRequestDto("feature/x")).worktree) + val nested = assertNotNull(api.create(parent.path, CreateWorktreeRequestDto("feature/y")).worktree) + val old = Path.of(parent.path).resolve(".kilo").resolve("worktrees").resolve("nested") + Files.createDirectories(old.parent) + git(parent.path, "worktree", "move", nested.path, old.toString()) + + val result = api.remove(repo.toString(), parent.path, parent.branch) + + assertFalse(result.ok) + assertTrue(result.error?.contains(old.toString()) == true, "error should name the blocker: ${result.error}") + assertTrue(Files.isDirectory(Path.of(parent.path))) + assertTrue(Files.isDirectory(old)) + } + + @Test + fun `remove succeeds when nested worktree directory is already gone`() = runBlocking { + initRepo() + val parent = assertNotNull(api.create(repo.toString(), CreateWorktreeRequestDto("feature/x")).worktree) + val nested = assertNotNull(api.create(parent.path, CreateWorktreeRequestDto("feature/y")).worktree) + val old = Path.of(parent.path).resolve(".kilo").resolve("worktrees").resolve("nested") + Files.createDirectories(old.parent) + git(parent.path, "worktree", "move", nested.path, old.toString()) + delete(old) + + val result = api.remove(repo.toString(), parent.path, parent.branch) + + assertTrue(result.ok, "remove should succeed despite dead nested metadata: ${result.error}") + assertFalse(Files.exists(Path.of(parent.path))) + } + + @Test + fun `remove prunes dangling metadata on success`() = runBlocking { + initRepo() + val dead = assertNotNull(api.create(repo.toString(), CreateWorktreeRequestDto("dead")).worktree) + delete(Path.of(dead.path)) + val live = assertNotNull(api.create(repo.toString(), CreateWorktreeRequestDto("live")).worktree) + + val result = api.remove(repo.toString(), live.path, live.branch) + + assertTrue(result.ok, "remove should succeed: ${result.error}") + val out = output(repo, "worktree", "list", "--porcelain") + assertFalse(out.contains(dead.path), "remove should prune unrelated dangling worktree metadata") + } + + @Test + fun `list drops missing worktrees and reconciles stored state`() = runBlocking { + initRepo() + val live = assertNotNull(api.create(repo.toString(), CreateWorktreeRequestDto("live")).worktree) + val dead = assertNotNull(api.create(repo.toString(), CreateWorktreeRequestDto("dead")).worktree) + assertNotNull(api.rename(repo.toString(), live.path, "Live").worktree) + assertNotNull(api.rename(repo.toString(), dead.path, "Dead").worktree) + delete(Path.of(dead.path)) + + val listed = api.list(repo.toString()).worktrees + + assertTrue(listed.any { it.path == live.path }) + assertFalse(listed.any { it.path == dead.path }) + val state = readWorktreeState(repo.resolve(".kilo").resolve("jetbrains.json")) + assertEquals(mapOf(live.path to "Live"), state.names) + assertEquals(listOf(live.path), state.worktreeOrder) + } + @Test fun `listBranches returns local branches and the current one`() = runBlocking { initRepo() @@ -592,6 +755,17 @@ class KiloWorktreeRpcApiImplTest { assertEquals(0, out.exitCode, "git ${args.joinToString(" ")} failed: ${out.stderr}") } + private fun git(dir: String, vararg args: String) { + git(Path.of(dir), *args) + } + + private fun output(dir: Path, vararg args: String): String { + val cmd = GeneralCommandLine(listOf("git") + args).withWorkDirectory(dir.toFile()) + val out = CapturingProcessHandler(cmd).runProcess(30_000) + assertEquals(0, out.exitCode, "git ${args.joinToString(" ")} failed: ${out.stderr}") + return out.stdout + } + private fun delete(dir: Path) { if (!Files.exists(dir)) return Files.walk(dir).use { paths -> diff --git a/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspaceTest.kt b/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspaceTest.kt index b3358b033b..93227e5254 100644 --- a/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspaceTest.kt +++ b/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspaceTest.kt @@ -25,6 +25,8 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout import kotlinx.coroutines.withTimeoutOrNull +import java.nio.file.Files +import java.nio.file.Path import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertEquals @@ -39,6 +41,8 @@ class KiloBackendWorkspaceTest { private val log = TestLog() private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) private val apps = mutableListOf() + private val root: Path = Files.createTempDirectory("kilo-backend-workspace") + private val project: Path = Files.createDirectories(root.resolve("project")) @AfterTest fun tearDown() { @@ -48,6 +52,7 @@ class KiloBackendWorkspaceTest { scope.cancel() mock.close() withTimeout(10_000) { scope.coroutineContext[Job]?.join() } + delete(root) } } @@ -69,9 +74,11 @@ class KiloBackendWorkspaceTest { private suspend fun ready(app: KiloBackendAppService): KiloBackendWorkspace { connect(app) - return app.workspaces.get("/test/project") + return app.workspaces.get(project.toString()) } + private fun dir(name: String): String = Files.createDirectories(root.resolve(name)).toString() + private suspend fun loaded(ws: KiloBackendWorkspace) { withTimeout(15_000) { ws.state.first { it is KiloWorkspaceState.Ready } @@ -112,8 +119,9 @@ class KiloBackendWorkspaceTest { val app = setup() connect(app) - val ws1 = app.workspaces.get("/test") - val ws2 = app.workspaces.get("/test") + val path = dir("same") + val ws1 = app.workspaces.get(path) + val ws2 = app.workspaces.get(path) // LLM note: get() starts background loading; settle it so teardown is not racing active HTTP calls in CI. loaded(ws1) assertTrue(ws1 === ws2) @@ -124,14 +132,16 @@ class KiloBackendWorkspaceTest { val app = setup() connect(app) - val ws1 = app.workspaces.get("/project-a") - val ws2 = app.workspaces.get("/project-b") + val first = dir("project-a") + val second = dir("project-b") + val ws1 = app.workspaces.get(first) + val ws2 = app.workspaces.get(second) // LLM note: get() starts background loading; settle both loads before the scope-cancelling teardown. loaded(ws1) loaded(ws2) assertTrue(ws1 !== ws2) - assertEquals("/project-a", ws1.directory) - assertEquals("/project-b", ws2.directory) + assertEquals(first, ws1.directory) + assertEquals(second, ws2.directory) } @Test @@ -150,7 +160,7 @@ class KiloBackendWorkspaceTest { // Manager should throw since app is disconnected assertFailsWith { - app.workspaces.get("/test/project") + app.workspaces.get(project.toString()) } } @@ -187,7 +197,7 @@ class KiloBackendWorkspaceTest { connect(app) // get() creates workspace and starts loading immediately - val ws = app.workspaces.get("/test") + val ws = app.workspaces.get(dir("plain")) withTimeout(15_000) { ws.state.first { it is KiloWorkspaceState.Ready } @@ -211,7 +221,7 @@ class KiloBackendWorkspaceTest { val err = ws.state.value as KiloWorkspaceState.Error assertTrue(err.message.contains("providers")) assertTrue(err.errors.any { it.resource == "providers" }) - assertTrue(log.messages.any { it.contains("Workspace error [/test/project]: Failed to load:") && it.contains("providers") }) + assertTrue(log.messages.any { it.contains("Workspace error [${project}]: Failed to load:") && it.contains("providers") }) } @Test @@ -298,6 +308,26 @@ class KiloBackendWorkspaceTest { assertEquals(0, mock.requestCount("/agent")) } + @Test + fun `missing directory transitions to Missing without fetching workspace data`() = runBlocking { + val app = setup() + connect(app) + mock.resetCounts() + val dir = Files.createTempDirectory("kilo-missing-workspace") + Files.delete(dir) + val ws = app.workspaces.get(dir.toString()) + + val state = withTimeout(15_000) { + ws.state.first { it is KiloWorkspaceState.Missing } + } as KiloWorkspaceState.Missing + + assertEquals(dir.toString(), state.path) + assertEquals(0, mock.requestCount("/agent")) + assertEquals(0, mock.requestCount("/provider")) + assertEquals(0, mock.requestCount("/command")) + assertEquals(0, mock.requestCount("/skill")) + } + @Test fun `commands failure transitions to Error`() = runBlocking { mock.commandsStatus = 500 @@ -446,7 +476,7 @@ class KiloBackendWorkspaceTest { @Test fun `workspace exposes sessions for its directory`() = runBlocking { mock.sessions = """[ - {"id":"ses_1","slug":"s","projectID":"p","directory":"/test/project","title":"T","version":"1","time":{"created":1,"updated":1}} + {"id":"ses_1","slug":"s","projectID":"p","directory":"${project}","title":"T","version":"1","time":{"created":1,"updated":1}} ]""" val app = setup() val ws = ready(app) @@ -460,7 +490,7 @@ class KiloBackendWorkspaceTest { @Test fun `workspace maps missing session timestamps to zero`() = runBlocking { mock.sessions = """[ - {"id":"ses_1","slug":"s","projectID":"p","directory":"/test/project","title":"T","version":"1","time":{"created":null,"updated":null}} + {"id":"ses_1","slug":"s","projectID":"p","directory":"${project}","title":"T","version":"1","time":{"created":null,"updated":null}} ]""" val app = setup() val ws = ready(app) @@ -473,14 +503,14 @@ class KiloBackendWorkspaceTest { @Test fun `workspace creates session in its directory`() = runBlocking { - mock.sessionCreate = """{"id":"ses_new","slug":"n","projectID":"p","directory":"/test/project","title":"New","version":"1","time":{"created":1,"updated":1}}""" + mock.sessionCreate = """{"id":"ses_new","slug":"n","projectID":"p","directory":"${project}","title":"New","version":"1","time":{"created":1,"updated":1}}""" val app = setup() val ws = ready(app) loaded(ws) val session = ws.createSession() assertEquals("ses_new", session.id) - assertEquals("/test/project", session.directory) + assertEquals(project.toString(), session.directory) } // ------ Concurrency tests ------ @@ -498,7 +528,7 @@ class KiloBackendWorkspaceTest { try { val results = (1..10).map { async(Dispatchers.Default) { - manager.get("/same/dir") + manager.get(dir("same-concurrent")) } }.awaitAll() @@ -572,7 +602,7 @@ class KiloBackendWorkspaceTest { ) withTimeout(15_000) { reload.await() } - val ws = app.workspaces.get("/test/project") + val ws = app.workspaces.get(project.toString()) assertTrue(ws !== initial) val state = withTimeout(15_000) { ws.state.first { @@ -646,4 +676,11 @@ class KiloBackendWorkspaceTest { {"name":"test-skill","description":"A test skill","location":"file:///test","content":"# Test"} ]""".trimIndent() } + + private fun delete(dir: Path) { + if (!Files.exists(dir)) return + Files.walk(dir).use { paths -> + paths.sorted(Comparator.reverseOrder()).forEach { Files.deleteIfExists(it) } + } + } } diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/session/controller/SessionController.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/session/controller/SessionController.kt index d64c4c7767..0772c36272 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/session/controller/SessionController.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/session/controller/SessionController.kt @@ -2411,6 +2411,14 @@ class SessionController( ) } + if (workspace.status == KiloWorkspaceStatusDto.MISSING) { + return SessionControllerEvent.ConnectionChanged.ShowError( + KiloBundle.message("session.connection.missing"), + KiloBundle.message("session.connection.missing.detail", workspace.error ?: directory), + "workspace", + ) + } + if (app.status == KiloAppStatusDto.READY && workspace.status == KiloWorkspaceStatusDto.READY && app.warnings.isNotEmpty()) { return SessionControllerEvent.ConnectionChanged.ShowWarning( summary(app.warnings.size), diff --git a/packages/kilo-jetbrains/frontend/src/main/resources/messages/KiloBundle.properties b/packages/kilo-jetbrains/frontend/src/main/resources/messages/KiloBundle.properties index aeff0f994a..a12c21fb71 100644 --- a/packages/kilo-jetbrains/frontend/src/main/resources/messages/KiloBundle.properties +++ b/packages/kilo-jetbrains/frontend/src/main/resources/messages/KiloBundle.properties @@ -13,6 +13,8 @@ session.connection.downloading.version=Downloading Kilo Core v{0} ({1})… {2}% session.connection.error.app=Connection failed session.connection.error.workspace=Workspace loading failed session.connection.error.unknown=Unknown error +session.connection.missing=Workspace folder missing +session.connection.missing.detail=Kilo can''t load this session because the workspace folder no longer exists: {0} session.connection.retry=Try again session.connection.unsupported=Workspace not supported session.connection.unsupported.devcontainer=Kilo runs on your host machine, so it can't reach the files inside this Dev Container. diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/WorktreeControllerTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/WorktreeControllerTest.kt index 9e1aca1bc6..f1030f2022 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/WorktreeControllerTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/WorktreeControllerTest.kt @@ -195,6 +195,25 @@ class WorktreeControllerTest : BasePlatformTestCase() { assertTrue(failures.first().locked) } + fun `test refused nested remove keeps the row and surfaces the error`() { + val item = WorktreeDto("/repo/.kilo/worktrees/feature-x", "feature-x", "feature/x", "/repo/.kilo/worktrees/feature-x") + rpc.listed += item + rpc.removeResult = { _, _, _ -> RemoveWorktreeResultDto(error = "Delete nested worktrees first:\n/repo/.kilo/worktrees/feature-x/.kilo/worktrees/nested") } + val controller = controller() + controller.reload() + flush() + + val failures = mutableListOf() + controller.remove(controller.model.getElementAt(0), onFailure = { failures.add(it) }) + flush() + + assertEquals(1, controller.model.size) + assertEquals("feature/x", controller.model.getElementAt(0).branch) + assertNull(controller.progress(item.id)) + assertEquals(listOf(false), rpc.removeForces.toList()) + assertEquals("Delete nested worktrees first:\n/repo/.kilo/worktrees/feature-x/.kilo/worktrees/nested", failures.single().error) + } + fun `test force remove passes the force flag and drops the row on success`() { val item = WorktreeDto("/repo/.kilo/worktrees/feature-x", "feature-x", "feature/x", "/repo/.kilo/worktrees/feature-x", locked = true) rpc.listed += item diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/session/controller/ConnectionDelayTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/session/controller/ConnectionDelayTest.kt index e64333062a..8d930b6077 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/session/controller/ConnectionDelayTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/session/controller/ConnectionDelayTest.kt @@ -193,6 +193,30 @@ class ConnectionDelayTest : SessionControllerTestBase() { ) } + fun `test missing workspace status shows missing folder message`() { + appRpc.state.value = KiloAppStateDto(KiloAppStatusDto.READY) + projectRpc.state.value = workspaceReady() + val m = controller(displayMs = 50) + val events = collect(m) + flush() + events.clear() + + projectRpc.state.value = KiloWorkspaceStateDto( + status = KiloWorkspaceStatusDto.MISSING, + error = "/repo/.kilo/worktrees/deleted", + ) + pause(80) + + val event = events.filterIsInstance().single() + assertEquals("Workspace folder missing", event.summary) + assertEquals( + "Kilo can't load this session because the workspace folder no longer exists: /repo/.kilo/worktrees/deleted", + event.detail, + ) + assertEquals("workspace", event.source) + assertFalse(event.detail.orEmpty().contains("JetBrains Gateway")) + } + fun `test ready hides visible delayed connection banner immediately`() { appRpc.state.value = KiloAppStateDto(KiloAppStatusDto.READY) projectRpc.state.value = workspaceReady() diff --git a/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/KiloWorkspaceStateDto.kt b/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/KiloWorkspaceStateDto.kt index 487951a112..aaf238e2f6 100644 --- a/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/KiloWorkspaceStateDto.kt +++ b/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/KiloWorkspaceStateDto.kt @@ -8,6 +8,7 @@ enum class KiloWorkspaceStatusDto { LOADING, READY, UNSUPPORTED, + MISSING, ERROR, } diff --git a/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/WorktreeDto.kt b/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/WorktreeDto.kt index b3a98d6d58..54ee736293 100644 --- a/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/WorktreeDto.kt +++ b/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/WorktreeDto.kt @@ -11,6 +11,7 @@ data class WorktreeDto( val main: Boolean = false, // primary working tree — not deletable val locked: Boolean = false, // git worktree lock — blocks a plain remove val lockReason: String? = null, // optional reason recorded when the tree was locked + val prunable: Boolean = false, // git marks metadata stale because the directory is gone ) @Serializable From a6a6a3aca63765e18c711297821e40bf1a8143a4 Mon Sep 17 00:00:00 2001 From: kirillk Date: Tue, 25 Aug 2026 13:27:02 -0400 Subject: [PATCH 2/2] fix(jetbrains): resolve worktree paths and evict cache canonically --- .../backend/rpc/KiloWorktreeRpcApiImpl.kt | 13 ++++++---- .../workspace/KiloBackendWorkspaceManager.kt | 24 +++++++++++++++++-- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt index dc5d0b9e1f..b339ab36ea 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt @@ -371,10 +371,12 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { !it.main && samePath(it.path, path) && item.parent == storage } ?: return@withContext RemoveWorktreeResultDto(error = "Refusing to remove unmanaged worktree: $path") - val root = Path.of(path).normalize() + // Compare canonical (symlink-resolved) paths: on macOS the temp/repo root is a symlink + // (/var -> /private/var), so a raw startsWith against normalized porcelain paths would miss + // a live child and let `git worktree remove --force` delete it recursively. + val root = realPath(path) val nested = all.filter { - val item = Path.of(it.path).normalize() - !it.prunable && Files.isDirectory(item) && !samePath(it.path, path) && item.startsWith(root) + !it.prunable && Files.isDirectory(Path.of(it.path)) && !samePath(it.path, path) && realPath(it.path).startsWith(root) } if (nested.isNotEmpty()) { val names = nested.joinToString("\n") { it.path } @@ -387,7 +389,10 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { val unlock = runGit(base, "worktree", "unlock", target.path) if (!unlock.ok) LOG.info("worktree unlock skipped: path=$path exit=${unlock.exit} stderr=${unlock.stderr.trim()}") } - val res = if (target.prunable || !Files.isDirectory(Path.of(target.path))) { + // Only skip git's own removal when the checkout directory is actually gone. Git also flags a + // worktree prunable when its admin metadata is stale while the files remain; those must still + // be deleted so a later create of the same slug is not blocked by leftovers. + val res = if (!Files.isDirectory(Path.of(target.path))) { GitResult(0, "", "") } else { runGit(base, "worktree", "remove", "--force", target.path) diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspaceManager.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspaceManager.kt index 93ec913349..d7f5da9b11 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspaceManager.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspaceManager.kt @@ -8,6 +8,8 @@ import ai.kilocode.jetbrains.api.client.DefaultApi import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.SharedFlow import okhttp3.OkHttpClient +import java.nio.file.Files +import java.nio.file.Path import java.util.concurrent.ConcurrentHashMap /** @@ -74,8 +76,26 @@ class KiloBackendWorkspaceManager( } } - /** Remove a workspace (e.g. when a worktree is deleted). */ + /** + * Remove any cached workspace whose directory resolves to the same real path as [dir]. + * Callers pass git porcelain paths, while workspaces are often keyed by the resolved + * (`toRealPath`) path or the IDE base path, so an exact-string match would miss the entry + * and leave a deleted worktree cached as Ready — still producing backend errors. + */ fun remove(dir: String) { - workspaces.remove(dir)?.stop() + val target = canonical(dir) + workspaces.keys.filter { canonical(it) == target }.forEach { key -> + log.info("Removing cached workspace for $key") + workspaces.remove(key)?.stop() + } + } + + /** Resolve symlinks on the parent so `/var/...` and `/private/var/...` compare equal even after the leaf is deleted. */ + private fun canonical(dir: String): String { + val path = Path.of(dir).normalize() + val parent = path.parent ?: return path.toString() + val name = path.fileName ?: return path.toString() + val root = runCatching { if (Files.exists(parent)) parent.toRealPath() else parent }.getOrDefault(parent) + return root.resolve(name).toString() } }