fix: unify container shutdown logic to prevent orphaned resources #19
Conversation
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
Extract common shutdown sequence into shutdownContainer helper that: - Kills all multiplexer sessions before stopping container - Waits for sessions to terminate - Removes session logs - Uses retry logic for container stop (handles "Resource busy") - Optionally removes container after stopping Update Stop, Remove, and Recreate to use the new helper, ensuring consistent behavior across all container lifecycle operations. Also improve error handling throughout: - Create cleanup now uses stopContainerWithRetry and reports failures - CreateSession cleanup reports if session kill fails - Remove now properly handles worktree removal errors - All cleanup failures are included in error messages for visibility Previously, Remove was silently ignoring container stop/remove errors, which could leave orphaned containers when sessions were still active. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
jmgilman
added a commit
that referenced
this pull request
Jan 3, 2026
…ources Current behavior: Container shutdown logic was inconsistent across Stop, Remove, and Recreate operations. Remove was silently ignoring container stop/remove errors, which could leave orphaned containers when sessions were still active. Cleanup error handling was inconsistent and some failures were ignored. New behavior: Extract common shutdown sequence into shutdownContainer helper that kills all multiplexer sessions before stopping container, waits for sessions to terminate, removes session logs, uses retry logic for container stop, and optionally removes container. All operations now use consistent shutdown logic with proper error handling and reporting of cleanup failures. Closes: #19
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.
Summary
shutdownContainerhelper methodStop,Remove, andRecreateto use the unified helperProblem
The
Removemethod was silently ignoring container stop/remove errors, which could leave orphaned containers when multiplexer sessions were still active. This caused "Resource busy" errors that were never surfaced to the user.Additionally, there was inconsistent behavior across
Stop,Remove, andRecreate:StopandRecreateproperly killed sessions before stopping containersRemovedid not, leading to the "Resource busy" failuresSolution
Created a
shutdownContainerhelper that encapsulates the common shutdown sequence:All three methods now use this helper, ensuring consistent behavior.
Additional Improvements
Createcleanup now usesstopContainerWithRetryand reports failuresCreateSessioncleanup reports if session kill fails (prevents orphaned tmux sessions)Removenow properly handles worktree removal errors (fails on real errors, ignores "not found")Test plan
just check)🤖 Generated with Claude Code