fix(app): kill processes inside a worktree before removing it#1422
Open
chphch wants to merge 1 commit into
Open
fix(app): kill processes inside a worktree before removing it#1422chphch wants to merge 1 commit into
chphch wants to merge 1 commit into
Conversation
`removeWorktree` ran `git worktree remove --force` while a happy session could still be cwd'd inside the worktree — typically a session that was orphaned when its daemon restarted, so R2/daemon-driven stop never reached it. `--force` then deleted the directory out from under the live process, which kept its server socket open and re-reported the session as `active` indefinitely. The session could never be archived from the app: every `active:false` was immediately overwritten by the orphan's next heartbeat. Stop those processes before deleting the directory. `killProcessesInWorktree` matches by working directory (Linux: /proc/<pid>/cwd, macOS: lsof), anchored to the worktree dir or a subdir of it so siblings sharing a name prefix are spared, and canonicalises the path so a symlinked parent still matches. SIGTERM lets the happy CLI shut its session down cleanly. Best-effort — a failure here never blocks the removal.
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.
Removing a git worktree from the app (
removeWorktreeinsources/utils/worktree.ts) rangit worktree remove --forcewhile a happy session could still be cwd'd inside that worktree — most often a session that got orphaned when its daemon restarted, so the daemon-driven stop never reached it.--forcethen deleted the directory out from under the live process, which kept its server socket open and re-reported the session asactiveon every heartbeat. The session became impossible to archive from the app: eachactive: falsewas instantly overwritten by the orphan's next heartbeat. This change stops any process whose working directory is inside the worktree (SIGTERM, best-effort) before the directory is removed.The new
killProcessesInWorktreeis portable across the host OS (Linux reads/proc/<pid>/cwd, macOS falls back tolsof), canonicalises the worktree path (pwd -P) so a symlinked parent still matches, and anchors the match to the worktree dir or a subdir of it so a sibling that merely shares a name prefix is never touched. It resolveslsofby absolute path so a strippedPATHstill finds it, and never blocks the removal if anything goes wrong.Proof
Ran the exact
killProcessesInWorktreeshell — the macOS/lsofbranch — verbatim against a real process tree, handing it a symlinked worktree path to exercise canonicalisation:wt-sibling) → spared (anchoring — a shared name prefix is not a match)…/link/wtwas canonicalised to…/real/wt, solsof's physical paths matchedlsofwas not onPATH, so the/usr/sbin/lsoffallback was exercisedremoveWorktreecalls this immediately beforegit worktree remove --force(see the diff), so the orphaned session is shut down cleanly and can be archived from the app as expected.