Skip to content

fix(agent-manager): prevent PTYs escaping worktree cleanup - #13221

Merged
marius-kilocode merged 4 commits into
mainfrom
make-pty-processes-durable-across-reloads
Aug 21, 2026
Merged

fix(agent-manager): prevent PTYs escaping worktree cleanup#13221
marius-kilocode merged 4 commits into
mainfrom
make-pty-processes-durable-across-reloads

Conversation

@marius-kilocode

@marius-kilocode marius-kilocode commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Worktree deletion can race with Agent Manager terminal creation. A regular embedded terminal, Run script, or Setup script can finish creating after cleanup has listed existing PTYs, leaving a shell rooted in a deleted worktree.

This change keeps the fix limited to Agent Manager. Worktree deletion now blocks regular and script PTY creation, waits for in-flight creates, closes known terminals, and performs the existing backend PTY cleanup before deleting the worktree. The block remains active through disk deletion and is released in a finally path. Worktree state is removed only after disk cleanup succeeds, so cleanup failures remain retryable.

The CLI PTY registry, server shutdown behavior, public API, SDK, process-tree termination, and persisted session lifecycle are unchanged. Project catalog removal and backend session deletion remain separate follow-ups.

Comment thread packages/core/src/kilocode/pty/termination.ts
Comment thread packages/core/src/kilocode/pty/registry.ts
Comment thread packages/core/src/kilocode/pty/registry.ts Outdated
Comment thread packages/kilo-vscode/src/agent-manager/discard-worktree.ts Outdated
Comment thread packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts Outdated
Comment thread packages/kilo-vscode/src/agent-manager/provider-multi-version.ts Outdated
Comment thread packages/core/src/pty.ts
Comment thread packages/kilo-vscode/src/agent-manager/WorktreeManager.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (7 files)
  • packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts
  • packages/kilo-vscode/src/agent-manager/discard-worktree.ts
  • packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts
  • packages/kilo-vscode/src/agent-manager/provider-multi-version.ts
  • packages/kilo-vscode/src/agent-manager/pty-cleanup.ts
  • packages/kilo-vscode/src/agent-manager/worktree-importer.ts
  • packages/kilo-vscode/tests/unit/pty-cleanup.test.ts
Previous Review Summaries (4 snapshots, latest commit b08c421)

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

Previous review (commit b08c421)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (6 files)
  • packages/core/src/kilocode/pty/termination.ts
  • packages/core/test/kilocode/pty-termination.test.ts
  • packages/kilo-vscode/src/agent-manager/discard-worktree.ts
  • packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts
  • packages/kilo-vscode/src/agent-manager/provider-multi-version.ts
  • packages/kilo-vscode/tests/unit/pty-cleanup.test.ts

Previous review (commit d3b5a27)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts 79 Session-create rollback still lets removeWorktree reject the handler
packages/kilo-vscode/src/agent-manager/provider-multi-version.ts 198 Same uncaught removeWorktree after a successful removePtys
packages/kilo-vscode/src/agent-manager/discard-worktree.ts 23 Session delete now aborts disk cleanup on a failed worktree rollback
packages/core/src/kilocode/pty/termination.ts 202 Successful Windows taskkill returns without verifying the process is gone
Files Reviewed (39 files)
  • .changeset/strong-pty-cleanup.md
  • packages/core/src/kilocode/pty/registry.ts
  • packages/core/src/kilocode/pty/termination.ts - 1 issue
  • packages/core/src/pty.ts
  • packages/core/test/kilocode/pty-durability.test.ts
  • packages/core/test/kilocode/pty-termination.test.ts
  • packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts
  • packages/kilo-vscode/src/agent-manager/ScriptTerminalManager.ts
  • packages/kilo-vscode/src/agent-manager/SessionTerminalManager.ts
  • packages/kilo-vscode/src/agent-manager/WorktreeManager.ts
  • packages/kilo-vscode/src/agent-manager/discard-worktree.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/provider-multi-version.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/pty-cleanup.ts
  • packages/kilo-vscode/src/agent-manager/script-terminal-runtime.ts
  • packages/kilo-vscode/src/agent-manager/terminal-manager.ts
  • packages/kilo-vscode/src/agent-manager/terminal-routing.ts
  • packages/kilo-vscode/src/agent-manager/worktree-importer.ts
  • packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts
  • packages/kilo-vscode/tests/unit/agent-manager-terminal-routing.test.ts
  • packages/kilo-vscode/tests/unit/agent-manager-terminal-state.test.ts
  • packages/kilo-vscode/tests/unit/pty-cleanup.test.ts
  • packages/kilo-vscode/tests/unit/sandbox-bootstrap.test.ts
  • packages/kilo-vscode/tests/unit/script-terminal-manager.test.ts
  • packages/kilo-vscode/tests/unit/session-terminal-manager.test.ts
  • packages/kilo-vscode/tests/unit/worktree-manager.test.ts
  • packages/kilo-vscode/webview-ui/agent-manager/terminal/state.ts
  • packages/opencode/src/kilocode/server/httpapi/pty-cleanup.ts
  • packages/opencode/src/kilocode/worktree/pty-cleanup.ts
  • packages/opencode/src/kilocode/worktree/pty-lease.ts
  • packages/opencode/src/server/routes/instance/httpapi/middleware/authorization.ts
  • packages/opencode/src/server/routes/instance/httpapi/server.ts
  • packages/opencode/src/server/server.ts
  • packages/opencode/src/worktree/index.ts
  • packages/opencode/test/kilocode/worktree-pty-lease.test.ts
  • packages/opencode/test/project/worktree-remove.test.ts
  • packages/opencode/test/server/httpapi-listen.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 9c77473)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts 79 Session-create rollback still lets removeWorktree reject the handler
packages/kilo-vscode/src/agent-manager/provider-multi-version.ts 198 Same uncaught removeWorktree after a successful removePtys
packages/kilo-vscode/src/agent-manager/discard-worktree.ts 23 Session delete now aborts disk cleanup on a failed worktree rollback
packages/core/src/kilocode/pty/termination.ts 202 Successful Windows taskkill returns without verifying the process is gone
Files Reviewed (39 files)
  • .changeset/strong-pty-cleanup.md
  • packages/core/src/kilocode/pty/registry.ts
  • packages/core/src/kilocode/pty/termination.ts - 1 issue
  • packages/core/src/pty.ts
  • packages/core/test/kilocode/pty-durability.test.ts
  • packages/core/test/kilocode/pty-termination.test.ts
  • packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts
  • packages/kilo-vscode/src/agent-manager/ScriptTerminalManager.ts
  • packages/kilo-vscode/src/agent-manager/SessionTerminalManager.ts
  • packages/kilo-vscode/src/agent-manager/WorktreeManager.ts
  • packages/kilo-vscode/src/agent-manager/discard-worktree.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/provider-multi-version.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/pty-cleanup.ts
  • packages/kilo-vscode/src/agent-manager/script-terminal-runtime.ts
  • packages/kilo-vscode/src/agent-manager/terminal-manager.ts
  • packages/kilo-vscode/src/agent-manager/terminal-routing.ts
  • packages/kilo-vscode/src/agent-manager/worktree-importer.ts
  • packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts
  • packages/kilo-vscode/tests/unit/agent-manager-terminal-routing.test.ts
  • packages/kilo-vscode/tests/unit/agent-manager-terminal-state.test.ts
  • packages/kilo-vscode/tests/unit/pty-cleanup.test.ts
  • packages/kilo-vscode/tests/unit/sandbox-bootstrap.test.ts
  • packages/kilo-vscode/tests/unit/script-terminal-manager.test.ts
  • packages/kilo-vscode/tests/unit/session-terminal-manager.test.ts
  • packages/kilo-vscode/tests/unit/worktree-manager.test.ts
  • packages/kilo-vscode/webview-ui/agent-manager/terminal/state.ts
  • packages/opencode/src/kilocode/server/httpapi/pty-cleanup.ts
  • packages/opencode/src/kilocode/worktree/pty-cleanup.ts
  • packages/opencode/src/kilocode/worktree/pty-lease.ts
  • packages/opencode/src/server/routes/instance/httpapi/middleware/authorization.ts
  • packages/opencode/src/server/routes/instance/httpapi/server.ts
  • packages/opencode/src/server/server.ts
  • packages/opencode/src/worktree/index.ts
  • packages/opencode/test/kilocode/worktree-pty-lease.test.ts
  • packages/opencode/test/project/worktree-remove.test.ts
  • packages/opencode/test/server/httpapi-listen.test.ts

Fix these issues in Kilo Cloud

Previous review (commit c564e4a)

Status: 8 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 2
WARNING 5
SUGGESTION 1
Issue Details (click to expand)

CRITICAL

File Line Issue
packages/core/src/kilocode/pty/termination.ts 70 POSIX teardown hard-fails before any kill if ps is missing/slow/non-zero
packages/core/src/kilocode/pty/registry.ts 242 Failed teardown leaves the directory permanently blocked for new PTYs

WARNING

File Line Issue
packages/core/src/kilocode/pty/registry.ts 109 Failed shutdown can permanently disable the process-wide PTY registry
packages/kilo-vscode/src/agent-manager/discard-worktree.ts 48 Create-block is released before disk removal, so a new PTY can outlive the worktree
packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts 73 Session-create rollback no longer catches removeWorktree failures
packages/kilo-vscode/src/agent-manager/provider-multi-version.ts 192 Same uncaught removeWorktree after a successful removePtys
packages/core/src/pty.ts 120 Concurrent remove returns success without waiting for the in-flight teardown

SUGGESTION

File Line Issue
packages/kilo-vscode/src/agent-manager/WorktreeManager.ts 449 Unmanaged paths now throw before the missing-path fast path, which can make Delete a no-op forever
Files Reviewed (34 files)
  • .changeset/durable-agent-manager-terminals.md
  • packages/core/src/kilocode/pty/registry.ts - 2 issues
  • packages/core/src/kilocode/pty/termination.ts - 1 issue
  • packages/core/src/location-services.ts
  • packages/core/src/pty.ts - 1 issue
  • packages/core/test/kilocode/pty-durability.test.ts
  • packages/core/test/kilocode/pty-termination.test.ts
  • packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts
  • packages/kilo-vscode/src/agent-manager/ScriptTerminalManager.ts
  • packages/kilo-vscode/src/agent-manager/WorktreeManager.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/discard-worktree.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/provider-multi-version.ts - 1 issue
  • packages/kilo-vscode/src/agent-manager/pty-cleanup.ts
  • packages/kilo-vscode/src/agent-manager/script-terminal-runtime.ts
  • packages/kilo-vscode/src/agent-manager/terminal-manager.ts
  • packages/kilo-vscode/src/agent-manager/terminal-routing.ts
  • packages/kilo-vscode/src/agent-manager/worktree-importer.ts
  • packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts
  • packages/kilo-vscode/tests/unit/agent-manager-terminal-routing.test.ts
  • packages/kilo-vscode/tests/unit/agent-manager-terminal-state.test.ts
  • packages/kilo-vscode/tests/unit/pty-cleanup.test.ts
  • packages/kilo-vscode/tests/unit/sandbox-bootstrap.test.ts
  • packages/kilo-vscode/tests/unit/script-terminal-manager.test.ts
  • packages/kilo-vscode/tests/unit/worktree-manager.test.ts
  • packages/kilo-vscode/webview-ui/agent-manager/terminal/state.ts
  • packages/opencode/src/effect/app-runtime.ts
  • packages/opencode/src/kilocode/worktree/pty-cleanup.ts
  • packages/opencode/src/server/routes/instance/httpapi/server.ts
  • packages/opencode/src/server/server.ts
  • packages/opencode/src/worktree/index.ts
  • packages/opencode/test/server/httpapi-listen.test.ts
  • packages/opencode/test/server/httpapi-pty.test.ts
  • packages/server/src/routes.ts

Fix these issues in Kilo Cloud


Reviewed by grok-4.6 · Input: 56.5K · Output: 7.1K · Cached: 358.7K

Review guidance: REVIEW.md from base branch main

@marius-kilocode
marius-kilocode force-pushed the make-pty-processes-durable-across-reloads branch from c564e4a to ac6ecc8 Compare August 19, 2026 13:03
@marius-kilocode marius-kilocode changed the title fix(agent-manager): make PTY processes durable across reloads fix(agent-manager): harden PTY cleanup lifecycle Aug 19, 2026
@marius-kilocode
marius-kilocode force-pushed the make-pty-processes-durable-across-reloads branch from d3b5a27 to 8038220 Compare August 19, 2026 14:08
@marius-kilocode marius-kilocode changed the title fix(agent-manager): harden PTY cleanup lifecycle fix(agent-manager): prevent PTYs escaping worktree cleanup Aug 19, 2026
Comment thread packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts
Comment thread packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts

@WebReflection WebReflection left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it needs a rebase (plus green "light" in CI after) but looks good to me, we can ignore my comment.

@marius-kilocode
marius-kilocode merged commit 871b527 into main Aug 21, 2026
46 of 49 checks passed
@marius-kilocode
marius-kilocode deleted the make-pty-processes-durable-across-reloads branch August 21, 2026 11:48
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