Fix worktree leak on agent session archive (#325974)#326216
Open
adityajha77 wants to merge 2 commits into
Open
Fix worktree leak on agent session archive (#325974)#326216adityajha77 wants to merge 2 commits into
adityajha77 wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a Git worktree leak when agent sessions are archived by ensuring the session’s worktree is removed even if it contains uncommitted changes (addressing orphaned {repo}.worktrees/* directories accumulating over time).
Changes:
- Removes the “skip cleanup when dirty” guard in
WorktreeIsolation.cleanupWorktreeOnArchive, allowing forced worktree removal on archive. - Updates the unit test to assert a dirty worktree is still removed during archive.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/vs/platform/agentHost/node/shared/worktreeIsolation.ts | Removes the uncommitted-changes safety check so archiving triggers forced worktree cleanup. |
| src/vs/platform/agentHost/test/node/shared/worktreeIsolation.test.ts | Updates the archive test to expect removal even when hasUncommittedChanges is true. |
Comments suppressed due to low confidence (1)
src/vs/platform/agentHost/node/shared/worktreeIsolation.ts:614
cleanupWorktreeOnArchivenow force-removes the worktree regardless of uncommitted changes. Given this is a destructive behavior change, it would be helpful to leave a short in-code note here to prevent future maintainers from reintroducing the old “skip when dirty” behavior (the existing JSDoc above still describes the old clean/dirty semantics).
try {
await this._gitService.removeWorktree(repositoryRoot, worktreePath);
this._logService.info(`[${this._logLabel}:${sessionId}] Removed worktree '${worktreePath.fsPath}' on archive`);
} catch (error) {
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a Git worktree resource leak that occurs when archiving agent sessions (#325974).
Currently, when a session is marked "done" (archived) in the Agents window, the worktree is not deleted if it has uncommitted changes. Since agent changes are often uncommitted, this safety check causes orphaned worktrees to accumulate indefinitely in the
{repo}.worktreesfolder.To resolve this, we remove the safety check and aggressively clean up the worktree when a session is archived.
Changes
src/vs/platform/agentHost/node/shared/worktreeIsolation.tsdirty(viahasUncommittedChanges) and skipping the cleanup. The_cleanupWorktreeOnArchivemethod now proceeds to remove the worktree directly.src/vs/platform/agentHost/test/node/shared/worktreeIsolation.test.tsarchive skips removal when the worktree has uncommitted changesto assert that a dirty worktree is forcefully removed.removeWorktreeis successfully called on the mocked Git service and the directory is deleted.Verification Plan
Automated Tests
src/vs/platform/agentHost/test/node/shared/worktreeIsolation.test.tswill verify thatremoveCallsis equal to1and the worktree directory is removed even when uncommitted changes exist.