mosaic/stack
1
0
Fork 0
Code Issues 10 Pull Requests Actions Packages Projects Releases 2 Wiki Activity

[HIGH] Fix worktree cleanup error swallowing #268

New Issue
Closed
opened 2026-02-02 23:17:10 +00:00 by jason.woltje · 1 comment
jason.woltje commented 2026-02-02 23:17:10 +00:00
Owner
Copy Link

Priority: HIGH

Problem:
WorktreeManagerService.cleanupWorktree() catches all errors and only logs warnings. Callers assume cleanup succeeded when it may have completely failed.

File: apps/orchestrator/src/git/worktree-manager.service.ts:231-236

Hidden Errors:

  • Git worktree locked by another process
  • Filesystem permission errors
  • Disk full errors
  • Invalid repository path
  • Network mount unavailable

Impact:
Failed worktree cleanup leaves orphaned directories consuming disk space. Multiple failures can exhaust disk, causing subsequent operations to fail mysteriously.

Acceptance Criteria:

  • cleanupWorktree() throws error when removal fails
  • Callers handle cleanup failures explicitly
  • Error messages include actionable information
  • Logging uses ERROR level (not WARN)
  • Tests verify error propagation

Recommended Fix:

async cleanupWorktree(repoPath: string, agentId: string, taskId: string): Promise<void> {
  const worktreePath = this.getWorktreePath(repoPath, agentId, taskId);
  
  try {
    await this.removeWorktree(worktreePath);
    this.logger.log(`Successfully cleaned up worktree for agent ${agentId}`);
  } catch (error) {
    const errorMsg = `Failed to cleanup worktree for agent ${agentId}: ${error.message}`;
    this.logger.error(errorMsg, { agentId, taskId, worktreePath });
    
    // Rethrow so caller knows cleanup failed
    throw new Error(errorMsg);
  }
}

Code Review Confidence: 85%
Found by: pr-review-toolkit:silent-failure-hunter

**Priority:** HIGH **Problem:** WorktreeManagerService.cleanupWorktree() catches all errors and only logs warnings. Callers assume cleanup succeeded when it may have completely failed. **File:** `apps/orchestrator/src/git/worktree-manager.service.ts:231-236` **Hidden Errors:** - Git worktree locked by another process - Filesystem permission errors - Disk full errors - Invalid repository path - Network mount unavailable **Impact:** Failed worktree cleanup leaves orphaned directories consuming disk space. Multiple failures can exhaust disk, causing subsequent operations to fail mysteriously. **Acceptance Criteria:** - [ ] cleanupWorktree() throws error when removal fails - [ ] Callers handle cleanup failures explicitly - [ ] Error messages include actionable information - [ ] Logging uses ERROR level (not WARN) - [ ] Tests verify error propagation **Recommended Fix:** ```typescript async cleanupWorktree(repoPath: string, agentId: string, taskId: string): Promise<void> { const worktreePath = this.getWorktreePath(repoPath, agentId, taskId); try { await this.removeWorktree(worktreePath); this.logger.log(`Successfully cleaned up worktree for agent ${agentId}`); } catch (error) { const errorMsg = `Failed to cleanup worktree for agent ${agentId}: ${error.message}`; this.logger.error(errorMsg, { agentId, taskId, worktreePath }); // Rethrow so caller knows cleanup failed throw new Error(errorMsg); } } ``` **Code Review Confidence:** 85% **Found by:** pr-review-toolkit:silent-failure-hunter
jason.woltje added this to the M6-AgentOrchestration (0.0.6) milestone 2026-02-02 23:17:10 +00:00
jason.woltje added the orchestrator label 2026-02-02 23:17:10 +00:00
jason.woltje commented 2026-02-03 17:37:44 +00:00
Author
Owner
Copy Link

✅ Fixed: Added WorktreeCleanupResult interface with structured results. Errors are now properly tracked and reported.

✅ Fixed: Added WorktreeCleanupResult interface with structured results. Errors are now properly tracked and reported.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ai
AI/LLM integration
 api
Backend/API work
 api
auth
Authentication
 database
database
Database/schema work
 devops
Docker/deployment
 docs
Documentation
 frontend
graph
knowledge-module
migration
Data migration
 orchestrator
Orchestrator service (apps/orchestrator/)
 p0
Critical priority
 p1
High priority
 p2
Medium priority
 p3
Low priority
 performance
Performance work
 phase-1
phase-2
phase-3
phase-4
phase-5
plugin
MoltBot plugin work
 search
security
Security-related
 setup
Initial setup
 testing
Testing/QA
 web
Frontend work
No Label orchestrator
Milestone
No items
No Milestone M6-AgentOrchestration (0.0.6)
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaic/stack#268
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?