Skip to content

fix(jetbrains): make Agent Manager worktree deletion non-blocking and fault tolerant - #13887

Merged
kirillk merged 2 commits into
mainfrom
tidy-tundra
Sep 8, 2026
Merged

kirillk merged 2 commits into
mainfrom
tidy-tundra

Conversation

@kirillk

@kirillk kirillk commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Issue

Fixes #

No tracked issue — found and fixed while diagnosing a user-reported JetBrains Agent Manager worktree deletion failure from local logs.

Context

Deleting an Agent Manager worktree with a large node_modules blocked the remove RPC for 25–30s, because git worktree remove --force runs the recursive filesystem delete synchronously. While that delete was in flight, every other polling loop (stats, dirty, prStatus, branchStatus, the gh probe) kept spawning git/gh processes against the directory mid-delete, and one directory disappearing mid-command threw IllegalStateException: ... Unable to read current working directory, which cancelled every other worktree's result through the shared parallel() coroutine scope — so a single slow delete broke stats/dirty/PR badges for the whole Agent Manager list, which is what the user experienced as "deletion failed."

Six real deletions were traced end-to-end in the log: all of them eventually succeeded, but each blocked 18–31s and produced a wave of unrelated-looking crashes for the surviving worktrees during that window.

Implementation

  • Non-blocking removal: remove() now atomically renames the worktree to a .kilo-delete-<uuid> sibling (new WorktreeTrash service, matching the VS Code extension's WorktreeManager convention — same prefix, so either client sweeps the other's orphans), runs git worktree prune --expire now, deletes the branch, and returns success immediately. The actual recursive delete runs in the background on WorktreeTrash's own coroutine scope. A worktree that is locked without force skips staging entirely and falls through to the existing synchronous git worktree remove --force — a filesystem rename can't see or honor a git lock, and renaming a directory git still considers locked would leave its metadata permanently stuck as prunable-but-locked.
  • Poll isolation: every read path (sync/list/stats/dirty/prStatus/branchStatus/the gh probe) now checks WorktreeTrash's doomed-path registry and skips a worktree that's mid-removal, without caching the empty answer.
  • Failure isolation: stats/dirty's per-item work is now wrapped so one worktree throwing (e.g. its directory vanishing mid-command) returns a neutral entry instead of cancelling every sibling result via parallel()'s shared coroutineScope. badDir() now also recognizes git's own "Unable to read current working directory" message (from the log) and classifies it as an expected race (INFO) rather than a real fault (WARN).
  • Honest timeouts: CmdOut now carries a timeout flag, the fallback git worktree remove --force gets a 10-minute budget instead of the 30s default used for cheap queries, and a killed process reports "timed out" instead of a blank error.
  • Serialized mutations: git-mutating RPCs (create/import/remove/rename/adopt/reorder/session-list) are now serialized per repository with a Mutex, so concurrent calls can't interleave worktree list/remove/prune or race the .kilo/jetbrains.json read-modify-write. Read paths stay unlocked. moveToWorktree's rollback deliberately isn't nested under the same lock as its create, since Mutex isn't reentrant.
  • Logging: every rejection/outcome in remove() now logs with elapsed time and the removal mode (prune-only / rename / git fallback), so a slow delete is distinguishable from a hung one.

Screenshots / Video

N/A — backend/logging change, no UI change.

How to Test

Manual/local verification

  • Ran ./gradlew test for the whole packages/kilo-jetbrains plugin (backend, frontend, shared) — all green.
  • Ran ./gradlew typecheck for the plugin — passes.
  • Traced the original bug report's kilo.log/idea.log excerpts against the new code paths to confirm the fix addresses the exact observed exceptions and blocking durations.

Reviewer test steps

  1. In a JetBrains Agent Manager session, create a worktree and run bun install (or similar) in it so it has a large node_modules.
  2. Delete that worktree. The row should disappear in well under a second, and kilo.log should show worktree removed: ... mode=rename ms=<small>.
  3. While the background delete finishes, confirm other worktrees' stats/dirty/PR badges keep updating with no Git comparison failed stack traces in the log.
  4. Delete several worktrees back-to-back and confirm each completes without errors and .kilo/worktrees/ ends up with no leftover .kilo-delete-* directories.

Blocked checks and substitute verification

  • Did not launch a full sandbox IDE (runIdeSplitMode) for this change; substituted with the Gradle unit/integration test suite (984 backend tests + frontend tests, all real git repos in temp directories, no mocks) covering the rename/prune fast path, the locked-without-force fallback, per-item failure isolation via a corrupted index, doomed-path skip behavior for stats/dirty/branchStatus, and four concurrent removes leaving consistent worktree-list/state-file output.

Checklist

  • Issue linked above, or exception explained
  • Tests/verification described
  • Screenshots/video included for visual changes, or marked N/A
  • Changeset considered for user-facing changes — not applicable: packages/kilo-jetbrains/ is not part of the bun changesets system (.changeset/config.json's fixed group only covers kilo-code/@kilocode/cli); JetBrains has its own changelog process via the release-jetbrains release flow.
  • I personally reviewed the diff and can explain the changes, including any AI-assisted work.

Get in Touch

… fault tolerant

Deleting a worktree with a large node_modules blocked the remove RPC for
25-30s (git worktree remove --force runs the recursive delete
synchronously), and status polling running concurrently spawned git/gh
processes against the directory mid-delete, throwing
"Unable to read current working directory" and cancelling every other
worktree's stats/dirty/PR result through the shared parallel() scope.

remove() now atomically renames the worktree to a `.kilo-delete-<uuid>`
sibling (matching the VS Code extension's WorktreeManager convention),
prunes git's metadata, deletes the branch, and returns immediately while
the actual recursive delete runs in the background via a new WorktreeTrash
service. A worktree still considered locked without `force` skips staging
entirely and goes through the existing synchronous git remove, since a
filesystem rename cannot see or honor a git lock.

Every read path (stats, dirty, prStatus, branchStatus, the gh probe) now
skips a worktree WorktreeTrash reports as doomed, and per-item stats/dirty
failures are isolated so one bad worktree can no longer fail the whole
batch. Git-mutating RPCs (create/import/remove/rename/adopt/reorder/session
list) are now serialized per repository with a Mutex to stop concurrent
calls from interleaving `worktree list`/`remove`/`prune` or racing the
`.kilo/jetbrains.json` read-modify-write. Failures now log with elapsed
time and a timeout flag instead of a blank error.
@kilo-code-bot

kilo-code-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (10 files)
  • packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/diff/GitComparison.kt
  • packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt
  • packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/PrResolver.kt
  • packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/worktree/WorktreeTrash.kt
  • packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt
  • packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTrashTest.kt
  • packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/worktree/WorktreeTrashTest.kt
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinator.kt
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeController.kt
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusService.kt
Previous Review Summary (commit 05c1bb0)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 05c1bb0)

Status: 5 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 3
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/worktree/WorktreeTrash.kt 153 canonical() identity changes after stage(), so unmark() can leak the marked path
packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/worktree/WorktreeTrash.kt 98 jobs grows without bound and every list() poll can race an in-flight delete
packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt 632 prune-only path deletes the branch before unregistering the worktree

SUGGESTION

File Line Issue
packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/worktree/WorktreeTrash.kt 76 Files.move without ATOMIC_MOVE can copy instead of failing over to git remove
packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt 840 statsSafe/dirtySafe swallow IntelliJ ProcessCanceledException
Files Reviewed (10 files)
  • packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/diff/GitComparison.kt - 0 issues
  • packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt - 2 issues
  • packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/PrResolver.kt - 0 issues
  • packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/worktree/WorktreeTrash.kt - 3 issues
  • packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt - 0 issues
  • packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTrashTest.kt - 0 issues
  • packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/worktree/WorktreeTrashTest.kt - 0 issues
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinator.kt - 0 issues
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeController.kt - 0 issues
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusService.kt - 0 issues

Fix these issues in Kilo Cloud


Reviewed by grok-4.6 · Input: 157.9K · Output: 17.8K · Cached: 576.3K

Review guidance: REVIEW.md from base branch main

- WorktreeTrash keyed marks on a normalization-only path and recorded every
  identity the path resolved to at mark() time. Keying on toRealPath() meant
  unmark() recomputed a different key once the checkout had been renamed away,
  leaking the entry for the rest of the session and keeping a worktree later
  recreated at the same slug invisible to every poll.

- stage() now demands ATOMIC_MOVE. A plain Files.move degrades to
  copy-then-delete across filesystems, which would block the caller for a full
  tree copy and leave the original beside a half-built sibling instead of
  failing over to the synchronous git fallback the KDoc promises.

- Reaps and sweeps no longer accumulate: each job drops itself from the tracking
  list on completion, a sweep already scanning a storage directory is skipped
  rather than duplicated, and a tree another walker owns is left to it. Added
  visitFileFailed/DirectoryNotEmptyException handling so a concurrently vanishing
  entry no longer aborts the rest of the walk.

- remove() now runs `worktree prune --expire now` on every successful arm before
  `branch -D`. The prune-only arm previously deleted the branch while git still
  had the worktree registered, so the delete failed (logged only) and the
  trailing prune used the default three-month expiry, leaving both the branch and
  the registration behind.
@kirillk
kirillk merged commit cc3f87c into main Sep 8, 2026
23 checks passed
@kirillk
kirillk deleted the tidy-tundra branch September 8, 2026 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants