Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/orphan-worktree-cleanup-ui.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"kilo-code": patch
---

Move the Agent Manager leftover worktree folder notice to the top of the worktree list and rework its cleanup dialog to match VS Code, including an explanation of what the listed folders are before any bulk delete
6 changes: 6 additions & 0 deletions .changeset/worktree-orphan-cleanup-dialog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"kilo-code": minor
"@kilocode/kilo-jetbrains": minor
---

Leftover worktree folders now show a count and total size in the warning banner, with a "Resolve…" action that opens a dialog listing each folder's path, size, and whether it still holds a git checkout. While the total is still being measured the banner says so, and if the measurement fails it reports the count alone instead of claiming 0 B. Measuring stops as soon as you start a delete, or whenever the folder list changes, rather than walking folders that are on their way out, and then re-measures whatever is left. The dialog's explanation starts collapsed to its first paragraph behind a "Show more" link, and its checkboxes are drawn the way each IDE draws its own. Deletion runs in the background so the UI never freezes, folders that still contain a checkout are unchecked by default, and a completion notification reports how many were removed. Worktree deletion is also more thorough: JetBrains now tears down backend state and removes the snapshot repository for a deleted worktree the same way VS Code already does, and a directory that reappears immediately after deletion is cleaned up once more automatically.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,18 @@ class WorktreeTrash(private val cs: CoroutineScope) {
}
}

/** Deletes [temp] recursively on this service's own scope. Never throws into the caller. */
fun reap(temp: Path) {
track(cs.launch(Dispatchers.IO) { reapNow(temp) })
/**
* Deletes [temp] recursively on this service's own scope. Never throws into the caller.
*
* [onDone] runs after the reap attempt finishes (success or failure), still on this service's
* own IO dispatcher — callers use it for the post-reap reappearance guard: staging a worktree
* created a fresh sibling that vanished the moment the rename completed, but something can still
* recreate the original path while the recursive delete of its sibling is still in flight (a
* dev-run backend re-materializing `.kilo-dev`, for example), and only checking after the delete
* settles observes that reliably.
*/
fun reap(temp: Path, onDone: () -> Unit = {}) {
track(cs.launch(Dispatchers.IO) { reapNow(temp); onDone() })
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package ai.kilocode.backend.worktree.orphans

import ai.kilocode.log.KiloLog
import java.io.IOException
import java.nio.file.FileVisitResult
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.SimpleFileVisitor
import java.nio.file.attribute.BasicFileAttributes
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.isActive
import kotlinx.coroutines.withContext

private val LOG = KiloLog.create(OrphanTree::class.java)

/** Marker class for [KiloLog.create] — this file has no class of its own. */
private class OrphanTree

/**
* Apparent on-disk size of each leftover worktree directory under `.kilo/worktrees/`.
*
* No dependency on [ai.kilocode.backend.rpc.KiloWorktreeRpcApiImpl] or any other worktree machinery —
* this is a pure filesystem walk, callable from the RPC layer or a test. Mirrors VS Code's
* `orphans/size.ts`.
*
* Cancellation here has to be cooperative. The caller cancels this pass whenever the orphan set
* changes or a delete starts, but [Files.walkFileTree] is blocking and cannot be interrupted, so a
* cancelled coroutine on [Dispatchers.IO] would otherwise keep walking every remaining path. The loop
* re-checks between paths and the walk itself is handed an `isActive` probe.
*
* A path that fails to walk — permission error, disappeared mid-walk — is omitted from the result
* rather than failing the whole batch, so one bad orphan can never block the size of the rest.
*/
suspend fun orphanSizes(paths: List<String>): Map<String, Long> =
withContext(Dispatchers.IO) {
val result = mutableMapOf<String, Long>()
for (raw in paths) {
ensureActive()
val path = Path.of(raw).normalize()
// A directory that is gone (already removed, mid-delete elsewhere) is a failure to
// measure, not a size of zero — omitted the same as a walk that throws below.
if (!Files.isDirectory(path)) {
LOG.info("worktree orphan size skipped: path=$raw reason=missing")
continue
}
val size = runCatching { walkSize(path) { isActive } }.getOrElse { err ->
LOG.info("worktree orphan size skipped: path=$raw message=${err.message}")
null
} ?: continue
result[raw] = size
}
result
}

/**
* Apparent size of [root]: sum of regular-file sizes, never following symlinks.
*
* Answers null once [active] goes false, so a walk that stopped early contributes nothing rather
* than a partial sum that would be indistinguishable from a real measurement.
*/
private fun walkSize(root: Path, active: () -> Boolean): Long? {
if (!Files.isDirectory(root)) return 0
var total = 0L
var stopped = false
Files.walkFileTree(
root,
object : SimpleFileVisitor<Path>() {
override fun preVisitDirectory(dir: Path, attrs: BasicFileAttributes): FileVisitResult = step()

override fun visitFile(file: Path, attrs: BasicFileAttributes): FileVisitResult {
val next = step()
if (next == FileVisitResult.TERMINATE) return next
if (!attrs.isSymbolicLink) total += attrs.size()
return next
}

override fun visitFileFailed(file: Path, exc: IOException): FileVisitResult = FileVisitResult.CONTINUE

private fun step(): FileVisitResult {
if (active()) return FileVisitResult.CONTINUE
stopped = true
return FileVisitResult.TERMINATE
}
},
)
return if (stopped) null else total
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import ai.kilocode.rpc.dto.GhState
import ai.kilocode.rpc.dto.MoveStage
import ai.kilocode.rpc.dto.WorktreeDirtyDto
import ai.kilocode.rpc.dto.WorktreeDto
import ai.kilocode.rpc.dto.orphans.OrphanKind
import ai.kilocode.rpc.dto.orphans.OrphanRemoveResultDto
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonObject
import com.intellij.execution.configurations.GeneralCommandLine
Expand All @@ -30,11 +32,16 @@ import com.intellij.openapi.util.SystemInfo
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.async
import kotlinx.coroutines.cancel
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.toList
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.sync.Mutex
import java.util.concurrent.TimeUnit
import org.junit.jupiter.api.Assumptions.assumeFalse
import org.junit.jupiter.api.Assumptions.assumeTrue
import java.nio.file.Files
Expand Down Expand Up @@ -136,12 +143,182 @@ class KiloWorktreeRpcApiImplTest {

// Orphan paths are resolved the way git reports worktree paths (realpath), so they compare
// equal to the other DTOs' paths on a symlinked temp dir.
assertEquals(listOf(leftover.toRealPath().toString()), listed.orphans)
assertEquals(listOf(leftover.toRealPath().toString()), listed.orphans.map { it.path })
assertEquals(listOf(OrphanKind.LEFTOVER), listed.orphans.map { it.kind }, "a plain directory is a leftover, not a broken checkout")
assertTrue(listed.worktrees.any { it.path == created.path }, "a live worktree is not an orphan")
// Reported, never deleted: a leftover directory can hold files that exist nowhere else.
assertTrue(Files.isDirectory(leftover.resolve(".kilo-dev")))
}

@Test
fun `list classifies an orphan that still holds a git checkout as broken`() = runBlocking {
initRepo()
val broken = repo.resolve(".kilo").resolve("worktrees").resolve("broken")
Files.createDirectories(broken)
// A plain file named `.git` is what a linked worktree checkout actually has (it points back
// at the shared `.git` dir) — a directory named `.git` is the primary repo's own shape. Either
// is enough to call the leftover "still holds a checkout".
Files.writeString(broken.resolve(".git"), "gitdir: /nowhere\n")

val listed = api.list(repo.toString())

assertEquals(listOf(broken.toRealPath().toString()), listed.orphans.map { it.path })
assertEquals(listOf(OrphanKind.BROKEN), listed.orphans.map { it.kind })
}

@Test
fun `orphanSizes sums regular file sizes and never follows symlinks`() = runBlocking {
initRepo()
val orphan = repo.resolve(".kilo").resolve("worktrees").resolve("leftover")
Files.createDirectories(orphan)
Files.write(orphan.resolve("a.bin"), ByteArray(10))
Files.write(orphan.resolve("b.bin"), ByteArray(20))
val target = Files.createTempDirectory("kilo-orphan-symlink-target")
Files.write(target.resolve("outside.bin"), ByteArray(1_000))
runCatching { Files.createSymbolicLink(orphan.resolve("link"), target) }
try {
val sizes = api.orphanSizes(repo.toString(), listOf(orphan.toString()))
assertEquals(30L, sizes[orphan.toString()], "size must exclude the symlinked tree entirely")
} finally {
delete(target)
}
}

@Test
fun `orphanSizes omits a path that fails to walk instead of failing the batch`() = runBlocking {
initRepo()
val ok = repo.resolve(".kilo").resolve("worktrees").resolve("leftover")
Files.createDirectories(ok)
Files.write(ok.resolve("a.bin"), ByteArray(5))
val missing = repo.resolve(".kilo").resolve("worktrees").resolve("does-not-exist")

val sizes = api.orphanSizes(repo.toString(), listOf(ok.toString(), missing.toString()))

assertEquals(5L, sizes[ok.toString()])
assertFalse(sizes.containsKey(missing.toString()), "a path that could not be measured must be omitted, not zero")
}

/**
* The frontend cancels a size pass whenever the orphan set changes or a delete starts, and
* `Files.walkFileTree` cannot be interrupted — so the walk has to check cancellation itself rather
* than leave a cancelled coroutine walking every remaining path on Dispatchers.IO.
*
* Cancelling before the call is ever reached (`job.cancel()` then `withContext(job) { ... }`) would
* only prove that `withContext` on an already-dead job throws without entering the block — true of
* any suspend function and unrelated to the `ensureActive()`/`isActive` checks this is meant to
* cover. This instead cancels a walk that is demonstrably still running: a full, uncancelled pass
* over a large tree is timed first, then a second pass over the same tree is cancelled partway
* through and shown to finish in a small fraction of that baseline — which a walk with no
* cancellation checks could not do, since nothing would tell it to stop.
*/
@Test
fun `orphanSizes stops mid-walk once cancelled, instead of running to completion`() = runBlocking {
initRepo()
val big = repo.resolve(".kilo").resolve("worktrees").resolve("big")
Files.createDirectories(big)
repeat(20_000) { i -> Files.write(big.resolve("f$i.bin"), ByteArray(1)) }
val target = listOf(big.toString())

val baselineStart = System.nanoTime()
api.orphanSizes(repo.toString(), target)
val baseline = System.nanoTime() - baselineStart

val job = Job()
val cancelledStart = System.nanoTime()
val deferred = CoroutineScope(Dispatchers.IO + job).async { api.orphanSizes(repo.toString(), target) }
// A quarter of the uncancelled duration is comfortably inside the walk, not before or after it.
delay(TimeUnit.NANOSECONDS.toMillis(baseline / 4).coerceAtLeast(1))
job.cancel()
val outcome = runCatching { deferred.await() }
val elapsed = System.nanoTime() - cancelledStart

assertTrue(
outcome.exceptionOrNull() is CancellationException,
"a cancelled pass must not answer with sizes -> ${outcome.getOrNull()}",
)
assertTrue(
elapsed < baseline * 3 / 4,
"cancelling took ${elapsed / 1_000_000}ms against an uncancelled ${baseline / 1_000_000}ms " +
"-- a walk with no cancellation checks would not finish any faster than the baseline",
)
}

@Test
fun `removeOrphans deletes a validated leftover directory`() = runBlocking {
initRepo()
val orphan = repo.resolve(".kilo").resolve("worktrees").resolve("leftover")
Files.createDirectories(orphan.resolve("nested"))
Files.writeString(orphan.resolve("nested").resolve("file.txt"), "hi")
val real = orphan.toRealPath().toString()

val result = api.removeOrphans(repo.toString(), listOf(real))

assertEquals(listOf(OrphanRemoveResultDto(real, ok = true)), result.results)
assertFalse(Files.exists(orphan), "the trash-unavailable fallback must delete synchronously, not defer")
assertTrue(api.list(repo.toString()).orphans.isEmpty())
}

@Test
fun `removeOrphans refuses a path outside managed storage`() = runBlocking {
initRepo()
val outside = repo.resolve("outside")
Files.createDirectories(outside)

val result = api.removeOrphans(repo.toString(), listOf(outside.toString()))

val entry = result.results.single()
assertFalse(entry.ok)
assertTrue(entry.error?.contains("Refusing") == true, entry.error)
assertTrue(Files.isDirectory(outside), "unmanaged directory must not be touched")
}

@Test
fun `removeOrphans refuses a path that is actually a registered worktree`() = runBlocking {
initRepo()
val created = assertNotNull(api.create(repo.toString(), CreateWorktreeRequestDto("feature/x")).worktree)

val result = api.removeOrphans(repo.toString(), listOf(created.path))

val entry = result.results.single()
assertFalse(entry.ok)
assertTrue(entry.error?.contains("managed worktree") == true, entry.error)
assertTrue(Files.isDirectory(Path.of(created.path)), "a live managed worktree must not be touched")
}

@Test
fun `removeOrphans rejects a stale selection that is no longer an orphan`() = runBlocking {
initRepo()
val orphan = repo.resolve(".kilo").resolve("worktrees").resolve("leftover")
Files.createDirectories(orphan)
val real = orphan.toRealPath().toString()
// Someone else already cleaned it up between the dialog's scan and this call.
delete(orphan)

val result = api.removeOrphans(repo.toString(), listOf(real))

val entry = result.results.single()
assertFalse(entry.ok)
assertTrue(entry.error?.contains("No longer an orphan") == true, entry.error)
}

@Test
fun `removeOrphans reports independent results per path`() = runBlocking {
initRepo()
val ok = repo.resolve(".kilo").resolve("worktrees").resolve("leftover")
Files.createDirectories(ok)
val okReal = ok.toRealPath().toString()
val outside = repo.resolve("outside")
Files.createDirectories(outside)

val result = api.removeOrphans(repo.toString(), listOf(okReal, outside.toString()))

assertEquals(2, result.results.size)
assertTrue(result.results.first { it.path == okReal }.ok)
assertFalse(result.results.first { it.path == outside.toString() }.ok)
assertFalse(Files.exists(ok))
assertTrue(Files.isDirectory(outside))
}

@Test
fun `list and the polls agree on which worktrees exist`() = runBlocking {
initRepo()
Expand Down Expand Up @@ -417,6 +594,53 @@ class KiloWorktreeRpcApiImplTest {
assertFalse(listed.contains(created.path), "a poll with a free lock should prune: $listed")
}

@Test
fun `checkReappearance re-stages and reaps once when the original path came back`() = runBlocking {
val scope = CoroutineScope(Dispatchers.Default + SupervisorJob())
val trash = WorktreeTrash(scope)
val trashApi = KiloWorktreeRpcApiImpl(trash)
try {
val dir = repo.resolve("reappeared")
Files.createDirectories(dir)
// Simulate a reap that already settled (the sibling from the original stage is long gone)
// while something recreated the original path afterward — the exact race the guard exists
// to catch, made deterministic instead of racing a real background reap.
val temp = assertNotNull(trash.stage(dir))
trash.reap(temp)
trash.drain()
Files.createDirectories(dir)

trashApi.checkReappearance(dir.toString(), temp)
trash.drain()

assertFalse(Files.exists(dir), "a path that reappeared after reap must be staged and reaped once more")
} finally {
scope.cancel()
}
}

@Test
fun `checkReappearance does nothing when the original path did not come back`() = runBlocking {
val scope = CoroutineScope(Dispatchers.Default + SupervisorJob())
val trash = WorktreeTrash(scope)
val trashApi = KiloWorktreeRpcApiImpl(trash)
try {
val dir = repo.resolve("gone-for-good")
Files.createDirectories(dir)
val temp = assertNotNull(trash.stage(dir))
trash.reap(temp)
trash.drain()

trashApi.checkReappearance(dir.toString(), temp)
trash.drain()

assertFalse(Files.exists(dir))
assertEquals(0, trash.pending(), "nothing should have been staged again")
} finally {
scope.cancel()
}
}

@Test
fun `staleWorktrees excludes a worktree already marked doomed in WorktreeTrash`() {
val trash = WorktreeTrash(CoroutineScope(Dispatchers.Default + SupervisorJob()))
Expand Down
Loading
Loading