feat(agent-manager): add delete action to agent_manager tool - #13056
feat(agent-manager): add delete action to agent_manager tool#13056romanstetsenko wants to merge 2 commits into
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| if (!state.getWorktree(worktreeID)) { | ||
| throw new OrchestrationError("unknown_session", "The worktree is not managed by this Agent Manager workspace") | ||
| } | ||
| await this.options.delete(worktreeID, origin.directory) |
There was a problem hiding this comment.
WARNING: delete reports success even when the deletion was aborted
options.delete resolves identically for success and failure because onDeleteWorktree/deleteLifecycleWorktree return null on every path. When clearRun fails to stop a Run/Setup script terminal, deleteLifecycleWorktree aborts the deletion (posting an error only to the webview) and the worktree card, .kilo/worktrees/<name> directory, and branch all remain — yet the caller still receives { operation: "delete", deleted: true } and the tool prints "Deleted Agent Manager worktree …". The UI path has a human who sees the error toast; a driven agent or orchestration flow will act on a false success. The same applies if the worktree disappears between the pre-check above and the actual delete (lifecycle just logs and returns).
Consider failing the request when the worktree is still present after the await — state here is the same WorktreeStateManager instance the lifecycle mutates, and removeWorktree runs synchronously on the success path before the async disk removal:
await this.options.delete(worktreeID, origin.directory)
if (state.getWorktree(worktreeID)) {
throw new OrchestrationError("host_error", "The worktree could not be deleted (a Run/Setup script may still be running)")
}Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Addressed in 0500cda: deleteWorktree now throws host_error if the worktree is still present in state after the teardown await, so an aborted deletion (e.g. Run/Setup script terminal could not be stopped) is no longer reported as success. Added a bridge test covering the aborted case.
| */ | ||
| export function harden(permission: string, pattern: string, rule: Rule): Rule { | ||
| if (permission !== "agent_manager" || !["prompt", "stop", "move"].includes(pattern) || rule.action !== "allow") return rule | ||
| if (permission !== "agent_manager" || !["prompt", "stop", "move", "delete"].includes(pattern) || rule.action !== "allow") return rule |
There was a problem hiding this comment.
SUGGESTION: The doc comment above harden is now stale
It still reads "Prompting, stopping, or moving an existing Agent Manager session has an external side effect." — worth mentioning deleting so the enumeration matches the pattern list updated on this line.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Addressed in 0500cda: the harden doc comment now enumerates prompting, stopping, moving, and deleting.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Incremental review of Files Reviewed (incremental delta + re-verified files)
Previous Review Summaries (2 snapshots, latest commit 0500cda)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 0500cda)Status: No Issues Found | Recommendation: Merge Incremental review of Files Reviewed (3 files)
Previous review (commit e9ed0a3)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (14 files)
Reviewed by kimi-k3 · Input: 32.5K · Output: 4.5K · Cached: 177.4K Review guidance: REVIEW.md from base branch |
Add a delete action to the agent_manager tool so driven agents and orchestration flows can tear down their own Agent Manager worktrees. delete removes the worktree card, its .kilo/worktrees/<name> directory, and its local branch, mirroring the UI 'Delete worktree' button, given a worktreeID from action=list. - protocol/tool: DeleteRequest + DeleteResult, wired through host.request - bridge: handle delete via deleteLifecycleWorktree (extracted closeSession/deleteWorktree helpers to stay under lint complexity) - SDK types.gen.ts + openapi.json hand-mirrored to generator output (CI regenerates)
…mission doc Address review comments on the delete action.
0500cda to
c21c64f
Compare
1 participant
Summary
Adds a
deleteaction to theagent_managertool so driven agents and orchestration flows can tear down their own Agent Manager worktrees.deleteremoves the worktree card, its.kilo/worktrees/<name>directory, and its local branch — mirroring the UI "Delete worktree" button — given aworktreeIDfromaction=list.This is the programmatic equivalent of the UI button that today only exists behind the
agentManager.deleteWorktreemessage;stoponly ends a session and leaves the worktree, directory, and branch behind.Why
A core Agent Manager use case is fan-out (one worktree per task/approach). Creation is scriptable; teardown is not. Driven agents and orchestration flows cannot clean up their own worktrees without dropping to manual git/fs commands.
What changed
packages/opencode/src/kilocode/agent-manager/protocol.ts:DeleteRequest(operation: "delete",worktreeID) +DeleteResult; added to theRequest/Resultunions;matchesnow matches delete by operation.packages/opencode/src/kilocode/tool/agent-manager.ts:DeleteParams, wired intoParams/WireParams(actionenum +worktreeID), and an execute branch that asks theagent_manager/deletepermission then callshost.request({ operation: "delete", ... }).packages/opencode/src/kilocode/permission/agent-manager.ts:deleteadded to the hardened side-effect patterns (a broadagent_manager: allowno longer silently grants deletion).packages/opencode/src/kilocode/agent-manager/service.ts:matchesupdate for delete.packages/kilo-vscode/.../orchestration-bridge.ts(+orchestration-setup.ts,AgentManagerProvider.ts): handles thedeleteoperation throughdeleteLifecycleWorktree, resolving the project context by directory like the existingclosepath.closeSession``/deleteWorktree` extracted into helpers to stay under the lint complexity limit.packages/sdk/js/src/v2/gen/types.gen.ts+packages/sdk/openapi.json: hand-mirrored to the generator output (CI regenerates these).WorktreeStateManager), and updated schema/permission tests.Notes
removeStaleaction (for already-gone worktrees) from the issue was intentionally left out to keep scope to the core ask.bun install --ignore-scripts(the repopostinstallwrites.git/config, which was permission-denied here and is non-essential).🤖 Generated with Kilo CLI