fix(down): preserve slot on ecluse down --keep-worktree#32
Open
hefgi wants to merge 11 commits into
Open
Conversation
After ecluse down --keep-worktree, the session was removed from state.json, so the next ecluse up from inside the same worktree would allocate a fresh slot (e.g. slot 1 instead of slot 3), changing all ports. Fix: introduce a Stopped status. When --keep-worktree is used, the session stays in state.json with status=stopped, clearing all service runtime state (pid files, tmux session, port_overrides). The slot stays reserved. The next ecluse up finds the stopped entry, logs "restarting at same slot", and routes through the existing resume path (bring_up with reuse_worktree=true at the stored slot) — so ports are reallocated at the same slot and stay consistent with what was there before. ecluse ls shows stopped sessions as "<slug> (stopped)".
- Reject Stopped sessions in ensure_session_settled so env/shell/status/sync error with an actionable hint instead of surfacing stale ports - Preserve Stopped state on shutdown --keep-worktrees (mirror down) - Add State::mark_stopped helper; error loudly on missing slug instead of silently skipping the transition (down + shutdown) - Refresh stale status field comment; tighten stopped-status JSON assertion - Update SKILL.md/README soft-restart docs to reflect slot preservation
- Preserve Stopped status when a down/shutdown fails or is aborted, instead of forcing the session back to Active (cmd_down inline restore + restore_session) - Make new_op_id collision-proof within a process via an atomic counter - Assert identity fields survive mark_stopped; use .clear() for collections - Docs: --reuse-worktree not needed to resume a stopped session; note the (stopped) ls label in README known-limits
- Fix duplicate session entry on down teardown failure: mark_pending leaves the entry in place, so reuse restore_session (remove-then-add) instead of a bare add_session that duplicated the slug in state.json - Fix resumed Stopped session persisting as Stopped in cmd_up_resume Ok(None) arm (zero/all-running services): force Active so up success doesn't wedge env/shell/status behind the new Stopped guard - Clarify resolve_slug_from_args matches Stopped sessions intentionally - Add State test pinning the mark_pending remove-then-add invariant - Drop remaining stale --reuse-worktree from SKILL.md (3) and README (1)
- Fix resumed Stopped session persisting empty port_overrides: fall through to bring_up (re-probe ports) instead of the cheap Ok(None) early-return, since mark_stopped cleared the ports the Ok(None) path would have re-persisted - Skip bring_down (and its pre_down/post_down hooks) when down runs on an already-Stopped session — no services to tear down - mark_stopped now also clears process_manager and compose/overlay/subset provisioning fields so a Stopped entry holds no stale handles; test asserts it - Simplify restore_session status logic to a match, still guarding Pending
- Fix orphaned worktree: skipping bring_down on an already-Stopped session also skipped the git-worktree removal, so down --delete-worktree left the directory on disk. Remove the worktree explicitly in the already_stopped branch (cmd_down and cmd_shutdown) when the user isn't keeping it - cmd_shutdown now mirrors cmd_down's already-stopped guard: skip bring_down (and its pre_down/post_down hooks against dead ports) for Stopped sessions - Document that resolve_slug_and_branch intentionally matches Stopped sessions - debug_assert restore_session is never handed a live Pending entry
- Extract teardown_or_skip_stopped helper shared by cmd_down and cmd_shutdown, removing the duplicated already-stopped skip + worktree-removal logic so the two paths can't drift - CHANGELOG: document the new Stopped-session contract under [Unreleased] - README: soft-restart resumes at the same slot but re-probes ports (not guaranteed identical) — align with SKILL.md and the known-limits wording
- Add unit tests for the new Stopped-session guards: ensure_session_settled (active ok / pending + stopped rejected with actionable hints) and teardown_or_skip_stopped (keeps worktree when asked; removes it via a real git worktree otherwise — guards the --delete-worktree-orphan regression) - Fix misleading comment on cmd_up_resume Ok(None): only Active sessions reach it (resume_provision routes Stopped through bring_up), so the status-force is defensive, not load-bearing
- Add integration coverage for the stopped-session round-trip: down --keep-worktree marks Stopped + reserves the slot, ls shows (stopped), env errors with an up hint, and plain up resumes at the same slot with re-probed ports; plus down --delete-worktree on a Stopped session leaves no orphan - Remove an over-strict debug_assert in restore_session: taking over a crashed in-flight (Pending) op legitimately passes a Pending snapshot, which the match already settles to Active — the assert wrongly panicked in debug/tests - De-flake down_during_slow_up_does_not_resurrect_the_session: wait for the worktree to exist (not just the state entry) before the takeover, so --delete-worktree can't race ahead of worktree creation
- cmd_flush now routes teardown through teardown_or_skip_stopped, so a Stopped session no longer fires pre_down/post_down hooks against dead ports on flush - cmd_shutdown summary calls out kept-as-stopped sessions instead of counting them plainly as torn down (they persist in state with worktrees preserved) - resume of a stopped session whose kept worktree was hand-deleted now errors with an actionable recovery hint instead of the generic --reuse-worktree advice for a flag the user never passed - Tests: cover the actionable missing-worktree error; assert state is emptied (not just ls output) after delete-worktree on a stopped session
- Fix shutdown summary counts: partition into explicit removed + stopped tallies instead of deriving torn_down = total - failed.len(). This stops sessions skipped as already-gone or taken over by a concurrent command from being miscounted, and stops kept-as-stopped sessions being double-counted - cmd_down no longer prints a 'torn down' success line when another command took the session over mid-teardown — it now reports only the takeover warning - Add integration coverage for shutdown --keep-worktrees marking every session Stopped with slots reserved, worktrees on disk, and the kept note in output
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
1 participant
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
ecluse down --keep-worktreeremoved the session fromstate.json. On the nextecluse upfrom inside that worktree, slot allocation ran fresh and picked the lowest available slot (e.g. slot 1 instead of slot 3), changing all ports.StoppedSessionStatusvariant. When--keep-worktreeis used, the session stays in state withstatus = stopped, service runtime fields cleared (pid files, tmux session, port_overrides), and the slot reserved.ecluse upfrom inside a stopped worktree finds the entry, logs "restarting at same slot", and routes through the existing resume path — ports reallocated at the same slot, consistent with the previous session.ecluse lsshows<slug> (stopped)for stopped sessions.Test plan
ecluse down --keep-worktreeon a slot-3 session: verifystate.jsonstill has the entry withstatus = "stopped"and slot 3ecluse upfrom inside the kept worktree: verify slot 3 is used (not slot 1)ecluse lsshows(stopped)labelecluse down(full teardown, no keep): session still removed from state as beforeStoppedserialization, used-slot counting, andfind_session_mut