fix: skip session cleanup of a workspace still used by a live session#2487
Open
polymath-orchestrator wants to merge 3 commits into
Open
fix: skip session cleanup of a workspace still used by a live session#2487polymath-orchestrator wants to merge 3 commits into
polymath-orchestrator wants to merge 3 commits into
Conversation
…gentWrapper#68) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FWvyzpHEWWMQcVN5rK9TJh
…rapper#68) ao session cleanup keyed workspace-reclaim eligibility on the session's terminated state alone. Persistent/shared worktrees (the orchestrator's) are reused across respawn, so a terminated predecessor and a live successor can share one path — cleanup then deleted the live session's cwd out from under it. Cleanup now builds the set of workspace paths still occupied by a non-terminated session and skips (reporting the reason) any terminated session whose canonicalized path is in that set. Distinct-path terminated workspaces are still reclaimed as before. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FWvyzpHEWWMQcVN5rK9TJh
…er#68 review) Final-review (Copilot) caught that the shared-workspace guard continued before m.runtime.Destroy, so a terminated session skipped for a shared workspace also skipped its runtime teardown — leaking the predecessor's own lingering keep-alive shell until cleanup reran. The runtime handle is per-session (independent of the shared workspace dir and the live successor's handle), so teardown now runs before the workspace guard; only workspace.Destroy is gated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FWvyzpHEWWMQcVN5rK9TJh
Collaborator
|
@polymath-orchestrator can you resolve merge conflicts |
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
2 participants
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.
Problem
Manager.Cleanupreclaims the workspaces of terminated sessions, but it keys eligibility on the session's terminated state alone — never on whether the workspace path is still referenced by another, live session.Persistent/shared worktrees break that assumption. An orchestrator's worktree is reused across respawn, so a terminated predecessor session and a live successor session can point at the same
WorkspacePath.ao session cleanupthen tears down the directory out from under the live session — deleting its current working directory. In-flight commands whose cwd was the deleted dir fail silently.Fix
Before reclaiming,
Cleanupbuilds the set of workspace paths still occupied by any non-terminated session (liveWorkspacePaths) and skips any terminated session whose path is in that set, reporting it inSkippedwith the reasonworkspace in use by a live session. Paths are compared withfilepath.Cleanso trailing-slash /.-segment variants of the same directory still match. Terminated workspaces no live session references are reclaimed exactly as before.Runtime teardown is deliberately kept before the workspace guard: the runtime handle is per-session (independent of the shared workspace dir and of the live successor's handle), so a skipped predecessor's own lingering keep-alive shell is still reclaimed rather than leaked.
Tests
TestCleanup_SkipsWorkspaceStillReferencedByLiveSession— a terminated predecessor sharing a live successor's worktree is skipped (not reclaimed), its own runtime handle is still torn down, and the shared directory is preserved.TestCleanup_LiveWorkspaceGuardNormalizesPaths— trailing-slash /.-segment path variants still match.TestCleanup_ReclaimsUnsharedWhileSkippingShared— an adjacent terminated workspace no live session references is still reclaimed alongside a shared skip.go build ./...,go vet ./...,go test ./...all green.