diff --git a/CHANGELOG.md b/CHANGELOG.md index b47c135..8a0924d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Changed +- `ecluse down --keep-worktree` (and `shutdown --keep-worktrees`) no longer drops the session from state — it marks it `Stopped`, keeping the slot reserved so the next `ecluse up` resumes at the same slot instead of allocating a new one (which would change all ports). `ecluse up ` auto-detects a stopped session and resumes it (no `--reuse-worktree` needed), re-probing for free ports. `ecluse ls` shows ` (stopped)`. While stopped, `ecluse env`/`shell`/`status`/`sync` error with a hint to run `ecluse up`, instead of surfacing stale, no-longer-running port values. + ### Fixed - `ecluse down` in tmux mode now kills the entire pane process group, not just the pane's foreground shell. Previously, multi-level child chains (`sh → pnpm → node → vite`, plus anything that calls `setsid()` like Cloudflare workerd) survived as orphans adopted by `launchd`/`init`, holding their ports indefinitely. Each orphan held 4-8 ports; after a few `up`/`down` cycles the next `ecluse up` would silently land on a port already held by a zombie, serving a different worktree's content. The same TERM→KILL grace pattern that was applied to the nohup path in PR #18 now applies to tmux. (#30) - `ecluse flush` now sweeps every process whose cwd is inside a worktree (`lsof +d `) AND every listener on a configured port (`base_port + slot*slot_stride` and `extra_ports[].base_port + slot*slot_stride` across all `max_slots`), killing each with TERM→KILL grace. The flush confirmation prompt warns that editors/shells with files open in worktrees will be killed; `--yes` bypass for CI is unchanged. (#30) diff --git a/README.md b/README.md index 1ab7cb2..fb7be2d 100644 --- a/README.md +++ b/README.md @@ -172,8 +172,8 @@ ecluse up --skip api # skip api; start everything else **Soft restart** — tear down services without losing your worktree, then spin them up fresh: ```bash -ecluse down feat-foo --keep-worktree # services torn down, worktree + branch kept -ecluse up feat-foo --reuse-worktree # new slot, fresh ports, worktree reused +ecluse down feat-foo --keep-worktree # services torn down, worktree + branch kept, slot reserved +ecluse up feat-foo # resumes at the same slot; ports are re-probed (stopped session auto-detected) ``` **Port override** — pin a specific service to a port for this session (useful when the auto-assigned port conflicts with something ecluse can't detect): @@ -260,8 +260,8 @@ Hooks run as shell commands inside the worktree directory with all `.env.ecluse` **Ports are checked, not reserved.** ecluse finds a free port at `ecluse up` time and writes it to `.env.ecluse`. There is a small window between the check and when your process actually binds — if something else takes the port in between, the port in `.env.ecluse` will be wrong. The fix is to tear down and recreate the session: ```bash -ecluse down feat-foo --keep-worktree -ecluse up feat-foo --reuse-worktree +ecluse down feat-foo --keep-worktree # session shows as `feat-foo (stopped)` in `ecluse ls` until the next up +ecluse up feat-foo # resumes the stopped session and re-probes for free ports ``` Or pin a specific port manually: @@ -270,7 +270,7 @@ Or pin a specific port manually: ecluse up feat-foo --port api=4001 ``` -**Process management is spawn-and-kill only.** For `host` and `hybrid` modes, services with `command` are spawned on `up` and killed on `down`. ecluse does not monitor or restart crashed processes — `ecluse ls` warns if a nohup-managed process has died. For a fresh start, use `ecluse down feat-foo --keep-worktree && ecluse up feat-foo --reuse-worktree`. +**Process management is spawn-and-kill only.** For `host` and `hybrid` modes, services with `command` are spawned on `up` and killed on `down`. ecluse does not monitor or restart crashed processes — `ecluse ls` warns if a nohup-managed process has died. For a fresh start, use `ecluse down feat-foo --keep-worktree && ecluse up feat-foo` (the stopped session is auto-detected on the next `up`). **`command` only works if the app reads its port from the environment.** ecluse injects the full `.env.ecluse` contents (all `ECLUSE_*` vars, `PORT`, `port_env` aliases) directly into the spawned process environment — no separate sourcing needed. It cannot help if: - The port is **hardcoded in source code** — the app must be changed to read `$PORT`. diff --git a/skills/ecluse/SKILL.md b/skills/ecluse/SKILL.md index 4a3c246..bed7a03 100644 --- a/skills/ecluse/SKILL.md +++ b/skills/ecluse/SKILL.md @@ -264,7 +264,7 @@ The canonical fix for misbehaving services is `ecluse down` + `ecluse up`, not ` ```bash ecluse down --keep-worktree -ecluse up --reuse-worktree +ecluse up # stopped session auto-detected; resumes at the same slot ``` This tears down only **your** services and respawns them with the right env. Idempotent, safe under parallel sessions, never touches another agent's work. Reach for this 95% of the time. @@ -573,7 +573,7 @@ Persistent conflict: change `base_port` in the relevant `[[services]]` block, or ecluse down --keep-worktree # 2. Restart with ecluse spawning the services directly -ecluse up --reuse-worktree +ecluse up # stopped session auto-detected; resumes at the same slot # 3. Verify ports are correct ecluse status @@ -766,7 +766,7 @@ inspect it with `ecluse whose-pid ` and kill it manually if intended. What ecluse intentionally does not do in v0. These are design decisions, not bugs. -- **Ports are checked, not reserved** — ecluse finds a free port at `up` time and writes it to `.env.ecluse`. There is a small window between that check and when your process actually binds. If another process takes the port in between, the value in `.env.ecluse` will be wrong. Fix: `ecluse down feat-foo --keep-worktree` then `ecluse up feat-foo --reuse-worktree`, or pin a specific port with `--port name=value`. +- **Ports are checked, not reserved** — ecluse finds a free port at `up` time and writes it to `.env.ecluse`. There is a small window between that check and when your process actually binds. If another process takes the port in between, the value in `.env.ecluse` will be wrong. Fix: `ecluse down feat-foo --keep-worktree` then `ecluse up feat-foo` (stopped session auto-detected, re-probes for a free port), or pin a specific port with `--port name=value`. - **No process lifecycle management beyond spawn/kill** — ecluse can spawn native services on `up` (via `command` + `process_manager`) and kill them on `down`, but cannot auto-restart a crashed process. If a service dies, `ecluse up` (idempotent — slug auto-detected from cwd) starts only the downed services. `ecluse up --force` kills everything on allocated ports and restarts fresh. `ecluse ls` and `ecluse env` warn about dead nohup processes. - **`command` requires the app to expose a port entry point** — ecluse injects the full `.env.ecluse` contents (`PORT`, `ECLUSE_SLOT`, `ECLUSE_SLUG`, `ECLUSE_MODE`, all `ECLUSE__PORT` vars, and any `port_env` aliases) directly into the spawned process environment — no separate sourcing step needed. If the port is hardcoded or set in a config file, resolve it via `.ecluse.toml` first: pass it as a CLI flag (`command = "vite --port $ECLUSE_WEB_PORT"`), or use `port_env` to inject it under the var name the app already reads. Modifying app source code is the last resort — see [Port wiring](#port-wiring--exhaust-eclusetoml-options-before-touching-app-code) above. - **Mode is set at `init`, not re-detected on `up`** — to change: `ecluse init --mode ` @@ -913,11 +913,11 @@ ecluse status --quiet # exit-code only (0 = all up, 1 = any down) **Soft restart** — tear down services without losing the git worktree, then spin up fresh: ```bash -ecluse down feat-foo --keep-worktree # stops services, removes session from state, keeps worktree on disk -ecluse up feat-foo --reuse-worktree # allocates a new slot, skips worktree creation +ecluse down feat-foo --keep-worktree # stops services, marks the session Stopped (slot reserved), keeps worktree on disk +ecluse up feat-foo # resumes at the same slot (stopped session auto-detected; --reuse-worktree not needed) ``` -Use this when a service failed to bind after `up` and you want a fresh start without losing changes in the worktree. +Use this when a service failed to bind after `up` and you want a fresh start without losing changes in the worktree. While stopped, `ecluse env`, `ecluse shell`, and `ecluse status` on that session error with a hint to run `ecluse up` — they will not surface stale, no-longer-running ports. **Port override** — pin a service to a specific port for this session: diff --git a/src/main.rs b/src/main.rs index 9ab1467..fc8bdb1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -496,6 +496,121 @@ mod tests { let s = svc_status(true, true, true, Some(99999)); assert_eq!(status_str(&s), "\u{2717} wrong owner (PID 99999)"); } + + // ── Stopped-session guards ───────────────────────────────────────────────── + + fn test_config() -> config::Config { + config::Config { + mode: config::Mode::Host, + max_slots: 4, + prefix: "test".into(), + worktree_dir: ".ecluse/worktrees".into(), + app_label: "ecluse.role".into(), + app_label_value: "app".into(), + strict_port: false, + port_search_range: 10, + slot_stride: 1, + services: vec![], + hooks: config::HookConfig::default(), + inherit_env: vec![], + } + } + + fn session_with_status(status: state::SessionStatus) -> state::Session { + state::Session { + slug: "feat".into(), + mode: config::Mode::Host, + slot: 1, + branch: "feat".into(), + worktree_path: "/tmp/does-not-exist".into(), + status, + pending_op: None, + compose_project: None, + overlay_file: None, + overlay_files: vec![], + compose_overlays: vec![], + app_port: None, + started_at: "2026-01-01T00:00:00Z".into(), + port_overrides: std::collections::HashMap::new(), + process_manager: None, + tmux_session: None, + pid_files: vec![], + log_dir: None, + services_subset: None, + } + } + + #[test] + fn ensure_session_settled_allows_active() { + let s = session_with_status(state::SessionStatus::Active); + assert!(ensure_session_settled(&s).is_ok()); + } + + #[test] + fn ensure_session_settled_rejects_pending() { + let s = session_with_status(state::SessionStatus::Pending); + let err = ensure_session_settled(&s).unwrap_err().to_string(); + assert!(err.contains("in progress"), "got: {err}"); + } + + #[test] + fn ensure_session_settled_rejects_stopped_with_up_hint() { + let s = session_with_status(state::SessionStatus::Stopped); + let err = ensure_session_settled(&s).unwrap_err().to_string(); + assert!(err.contains("stopped"), "got: {err}"); + // Actionable: points the user at the command that revives it. + assert!(err.contains("ecluse up"), "got: {err}"); + } + + #[test] + fn teardown_skips_bring_down_for_stopped_and_keeps_worktree() { + // keep_worktree=true on a Stopped session: no bring_down, no removal, + // just Ok — the worktree stays on disk untouched. + let dir = tempfile::TempDir::new().unwrap(); + let wt = dir.path().join("kept"); + std::fs::create_dir(&wt).unwrap(); + let mut s = session_with_status(state::SessionStatus::Stopped); + s.worktree_path = wt.to_string_lossy().into_owned(); + let config = test_config(); + let log = log::StepLogger::new(true); + + teardown_or_skip_stopped(&s, &config, dir.path(), false, true, &log).unwrap(); + assert!(wt.exists(), "kept worktree must remain on disk"); + } + + #[test] + fn teardown_removes_worktree_for_stopped_when_not_kept() { + // keep_worktree=false on a Stopped session: bring_down is skipped but the + // worktree removal still runs. Use a real git repo + registered worktree + // so WorktreeManager::remove succeeds and the directory is deleted — + // guarding against the --delete-worktree-orphans-directory regression. + let dir = tempfile::TempDir::new().unwrap(); + let root = dir.path(); + let git = |args: &[&str]| { + let ok = std::process::Command::new("git") + .args(args) + .current_dir(root) + .status() + .unwrap() + .success(); + assert!(ok, "git {args:?} failed"); + }; + git(&["init", "-q"]); + git(&["config", "user.email", "t@t"]); + git(&["config", "user.name", "t"]); + git(&["commit", "-q", "--allow-empty", "-m", "init"]); + let wt = root.join("wt-gone"); + git(&["worktree", "add", "-q", wt.to_str().unwrap(), "-b", "gone"]); + assert!(wt.exists()); + + let mut s = session_with_status(state::SessionStatus::Stopped); + s.worktree_path = wt.to_string_lossy().into_owned(); + let config = test_config(); + let log = log::StepLogger::new(true); + + teardown_or_skip_stopped(&s, &config, root, false, false, &log).unwrap(); + assert!(!wt.exists(), "worktree must be removed when not kept"); + } } /// Sanitize a branch name or slug into a valid ecluse slug + original branch pair. @@ -569,6 +684,9 @@ fn resolve_slug_and_branch( let cwd = std::env::current_dir().context("could not determine current directory")?; // 1. Inside an ecluse-registered worktree → reuse stored slug/branch. + // Includes Stopped sessions so `ecluse up` from inside a kept worktree + // auto-detects the slug and resumes at the same slot — do not filter to + // Active here or the stopped-session resume flow breaks. if let Some(session) = state .sessions .iter() @@ -591,16 +709,25 @@ fn resolve_slug_and_branch( Ok((slug, branch, false, None)) } -/// Error when the session is mid-operation — its env and services are in flux. +/// Error unless the session is `Active` — its env and services are only +/// meaningful then. `Pending` means an op is in flight; `Stopped` means the +/// worktree was kept but services are down, so reading its env/status/shell +/// would surface stale slot values for services that are no longer running. fn ensure_session_settled(session: &state::Session) -> Result<()> { - if session.status == state::SessionStatus::Pending { - return Err(anyhow::anyhow!( + match session.status { + state::SessionStatus::Active => Ok(()), + state::SessionStatus::Pending => Err(anyhow::anyhow!( "session '{}' has an up/down operation in progress; retry when it finishes, or run `ecluse down {}` if it crashed", session.slug, session.slug - )); + )), + state::SessionStatus::Stopped => Err(anyhow::anyhow!( + "session '{}' is stopped (worktree kept at {}); run `ecluse up {}` to restart it", + session.slug, + session.worktree_path, + session.slug + )), } - Ok(()) } fn resolve_slug_from_args(arg: Option<&str>, state: &state::State, hint: &str) -> Result { @@ -612,7 +739,10 @@ fn resolve_slug_from_args(arg: Option<&str>, state: &state::State, hint: &str) - None => { let cwd = std::env::current_dir().context("could not determine current directory")?; - // Inside an active ecluse session — use it. + // Inside any known ecluse session — use it. Includes Stopped + // sessions so `ecluse up`/`down` resolve the slug from inside a + // kept worktree (read commands then reject Stopped via + // ensure_session_settled). if let Some(session) = state .sessions .iter() @@ -771,10 +901,17 @@ fn cmd_up(args: cli::UpArgs) -> Result<()> { } } log.step("Looking for existing session..."); - log.detail(&format!( - "found session '{}' (slot {}) — reusing worktree", - slug, existing.slot - )); + if existing.status == state::SessionStatus::Stopped { + log.detail(&format!( + "found stopped session '{}' (slot {}) — restarting at same slot", + slug, existing.slot + )); + } else { + log.detail(&format!( + "found session '{}' (slot {}) — reusing worktree", + slug, existing.slot + )); + } return cmd_up_resume(existing, args, config, root, guard, log); } @@ -1000,13 +1137,21 @@ fn cmd_up_resume( Ok(()) } Ok(None) => { - guard.state.add_session(existing.clone()); + // Nothing to start (all services already running). Only an Active + // session can reach here: resume_provision short-circuits Ok(None) + // for Stopped sessions (`!resuming_stopped`), routing them through + // bring_up instead so their ports get re-probed. So `existing` is + // already Active — just clear any stale pending_op and re-add it. + let mut restored = existing.clone(); + restored.status = state::SessionStatus::Active; + restored.pending_op = None; + guard.state.add_session(restored.clone()); guard.commit()?; log.step("All services already running — nothing to do."); if args.json { - print_up_json(&existing, &root)?; + print_up_json(&restored, &root)?; } else { - print_up_summary(&existing, &config, &log); + print_up_summary(&restored, &config, &log); } Ok(()) } @@ -1096,10 +1241,30 @@ fn resume_provision( let total = config.services.len(); let to_start = total.saturating_sub(skipped_count); - if to_start == 0 && !args.force { + // A Stopped session had its port_overrides/app_port cleared by + // `mark_stopped`. Even when nothing needs (re)starting, fall through to + // `bring_up` so it re-probes and re-fills the port mapping — otherwise the + // resumed Active session would be persisted with no ports and `ecluse env` + // would emit nothing. Active sessions keep the cheap early-return. + let resuming_stopped = existing.status == state::SessionStatus::Stopped; + if to_start == 0 && !args.force && !resuming_stopped { return Ok(None); } + // Resume always reuses the recorded worktree. If a Stopped session's kept + // worktree was deleted by hand, bring_up would surface the generic + // "remove --reuse-worktree" advice — nonsense here, since the user ran a bare + // `ecluse up`. Give a tailored, actionable message instead. + if resuming_stopped && !std::path::Path::new(&existing.worktree_path).exists() { + return Err(anyhow::anyhow!( + "worktree for stopped session '{}' is missing at {}; run `ecluse down {}` to clear the stale entry, then `ecluse up {}` to start fresh", + existing.slug, + existing.worktree_path, + existing.slug, + existing.slug + )); + } + let port_overrides: std::collections::HashMap = args.port_overrides.iter().cloned().collect(); let service_filter = parse_service_filter(&args.services, config)?; @@ -1329,8 +1494,7 @@ fn cmd_down(args: cli::DownArgs) -> Result<()> { } }; - let handler = modes::get_handler_for_mode(&session.mode); - let result = handler.bring_down( + let result = teardown_or_skip_stopped( &session, &config, &root, @@ -1339,27 +1503,37 @@ fn cmd_down(args: cli::DownArgs) -> Result<()> { &log, ); + if let Err(e) = result { + // Teardown failed — restore the session so it can be retried. Use the + // shared helper (remove-then-add under its own lock): `mark_pending` + // left the Pending entry in place, so a bare add_session would + // duplicate it. The helper also preserves a pre-existing Stopped status + // and is a no-op if another command took the session over. + restore_session(&root, &session, &op_id)?; + return Err(e); + } + let mut guard = state::StateGuard::acquire(&root)?; if guard.state.still_owned(&slug, &op_id) { - guard.state.remove_session(&slug); - if let Err(e) = result { - // Teardown failed — keep the session visible so it can be retried. - let mut restored = session; - restored.status = state::SessionStatus::Active; - restored.pending_op = None; - guard.state.add_session(restored); - guard.commit()?; - return Err(e); + if keep_worktree { + // Services are down but the worktree stays on disk. + // Mark Stopped so the next `ecluse up` from inside the worktree + // resumes at this slot rather than allocating a new one. + guard.state.mark_stopped(&slug)?; + } else { + guard.state.remove_session(&slug); } guard.commit()?; } else { // Another command took the session over during teardown — leave the - // entry to its new owner, but still report our own outcome. + // entry to its new owner. Teardown itself succeeded, but this invocation + // no longer owns the outcome, so report only the takeover and stop: + // printing "torn down" here would contradict the warning. drop(guard); log.warn(&format!( "session '{slug}' was taken over by another command during teardown; leaving its state entry alone" )); - result?; + return Ok(()); } if args.keep_branch { @@ -1383,10 +1557,12 @@ fn cmd_down(args: cli::DownArgs) -> Result<()> { Ok(()) } -/// Put a session back into state with Active status (used when an operation -/// that marked it Pending aborts or fails without changing anything durable). -/// No-op when `op_id` no longer owns the entry — another command took the -/// session over and restoring would clobber its work. +/// Restore a session that was marked Pending for an operation that then aborted +/// or failed without changing anything durable. A pre-existing Stopped status is +/// preserved (a `down`/`shutdown` on an already stopped session that bailed); +/// otherwise the entry settles back to Active. No-op when `op_id` no longer owns +/// the entry — another command took the session over and restoring would clobber +/// its work. fn restore_session(root: &std::path::Path, session: &state::Session, op_id: &str) -> Result<()> { let mut guard = state::StateGuard::acquire(root)?; if !guard.state.still_owned(&session.slug, op_id) { @@ -1394,12 +1570,54 @@ fn restore_session(root: &std::path::Path, session: &state::Session, op_id: &str } guard.state.remove_session(&session.slug); let mut restored = session.clone(); - restored.status = state::SessionStatus::Active; + // Restore the session's settled status: keep a pre-existing Stopped (a + // down/shutdown on an already-stopped session that then failed/aborted), but + // settle everything else to Active. `session` is the snapshot returned by + // `mark_pending`, so it can be Active, Stopped, OR Pending — the Pending case + // arises when we took over a crashed in-flight op, and it must become Active + // (never stay wedged Pending). + restored.status = match session.status { + state::SessionStatus::Stopped => state::SessionStatus::Stopped, + _ => state::SessionStatus::Active, + }; restored.pending_op = None; guard.state.add_session(restored); guard.commit() } +/// Tear down a session's services, or skip that when it is already Stopped. +/// +/// A Stopped session (from a prior `down --keep-worktree`) has no running +/// services and cleared runtime state, so calling `bring_down` would fire +/// pre_down/post_down hooks against nominal ports with nothing behind them. +/// We skip it — but `bring_down` is also the only place the worktree is +/// removed, so when the caller isn't keeping the worktree we remove it here +/// directly, or a `down --delete-worktree` on a Stopped session would orphan +/// the directory. Shared by `cmd_down` and `cmd_shutdown` so the two paths +/// can't drift. +fn teardown_or_skip_stopped( + session: &state::Session, + config: &config::Config, + root: &std::path::Path, + keep_volumes: bool, + keep_worktree: bool, + log: &log::StepLogger, +) -> Result<()> { + if session.status != state::SessionStatus::Stopped { + let handler = modes::get_handler_for_mode(&session.mode); + return handler.bring_down(session, config, root, keep_volumes, keep_worktree, log); + } + + log.detail("session already stopped — skipping service teardown"); + if !keep_worktree { + log.step("Removing worktree..."); + log.detail(&session.worktree_path); + worktree::WorktreeManager::new(root.to_owned()) + .remove(std::path::Path::new(&session.worktree_path))?; + } + Ok(()) +} + // ── shutdown ────────────────────────────────────────────────────────────────── fn cmd_shutdown(args: cli::ShutdownArgs) -> Result<()> { @@ -1422,11 +1640,17 @@ fn cmd_shutdown(args: cli::ShutdownArgs) -> Result<()> { let total = sessions.len(); let mut failed: Vec = Vec::new(); + // Outcome tallies that partition the sessions this run actually acted on: + // `removed` = torn down and dropped from state; `stopped` = torn down but + // kept as Stopped (worktree preserved). Counted explicitly rather than + // derived, so sessions skipped as already-gone or taken over by a concurrent + // command aren't miscounted as torn down. + let mut removed = 0usize; + let mut stopped = 0usize; for session in sessions { log.step(&format!("Tearing down '{}'...", session.slug)); log.detail(&format!("slot {}, mode: {}", session.slot, session.mode)); - let handler = modes::get_handler_for_mode(&session.mode); let keep_wt = match resolve_worktree_keep( std::path::Path::new(&session.worktree_path), @@ -1457,11 +1681,22 @@ fn cmd_shutdown(args: cli::ShutdownArgs) -> Result<()> { } }; - match handler.bring_down(¤t, &config, &root, args.keep_volumes, keep_wt, &log) { + let teardown = + teardown_or_skip_stopped(¤t, &config, &root, args.keep_volumes, keep_wt, &log); + match teardown { Ok(()) => { let mut guard = state::StateGuard::acquire(&root)?; if guard.state.still_owned(¤t.slug, &op_id) { - guard.state.remove_session(¤t.slug); + if keep_wt { + // Mirror `cmd_down`: the worktree stays on disk, so + // preserve the entry as Stopped to reserve the slot for + // the next `ecluse up` rather than dropping it. + guard.state.mark_stopped(¤t.slug)?; + stopped += 1; + } else { + guard.state.remove_session(¤t.slug); + removed += 1; + } guard.commit()?; } else { log.warn(&format!( @@ -1479,19 +1714,27 @@ fn cmd_shutdown(args: cli::ShutdownArgs) -> Result<()> { } println!(); - let torn_down = total - failed.len(); + // `removed` and `stopped` partition the sessions torn down by this run. + let torn_down = removed + stopped; + let kept_note = if stopped > 0 { + format!(" ({stopped} kept as stopped — worktrees preserved)") + } else { + String::new() + }; if failed.is_empty() { log.success(&format!( - "Shutdown complete — {} session{} torn down.", + "Shutdown complete — {} session{} torn down{}.", torn_down, - if torn_down == 1 { "" } else { "s" } + if torn_down == 1 { "" } else { "s" }, + kept_note )); } else { log.warn(&format!( - "{}/{} session{} torn down; {} failed: {}", + "{}/{} session{} torn down{}; {} failed: {}", torn_down, total, if total == 1 { "" } else { "s" }, + kept_note, failed.len(), failed.join(", ") )); @@ -1564,10 +1807,10 @@ fn cmd_ls(args: cli::LsArgs) -> Result<()> { pairs.join(" ") }; SessionRow { - slug: if s.status == state::SessionStatus::Pending { - format!("{} (pending)", s.slug) - } else { - s.slug.clone() + slug: match s.status { + state::SessionStatus::Pending => format!("{} (pending)", s.slug), + state::SessionStatus::Stopped => format!("{} (stopped)", s.slug), + state::SessionStatus::Active => s.slug.clone(), }, mode: s.mode.to_string(), slot: s.slot, @@ -2085,8 +2328,12 @@ fn cmd_flush(args: cli::FlushArgs) -> Result<()> { )); for session in sessions { log.detail(&format!(" down '{}'", session.slug)); - let handler = modes::get_handler_for_mode(&session.mode); - if let Err(e) = handler.bring_down(&session, &config, &root, false, false, &log) { + // Shared helper skips bring_down (and its hooks) for Stopped + // sessions with nothing running, and removes the worktree + // (flush always deletes worktrees) — same decision cmd_down uses. + if let Err(e) = + teardown_or_skip_stopped(&session, &config, &root, false, false, &log) + { log.warn(&format!( " '{}' teardown failed: {e} (continuing)", session.slug diff --git a/src/state.rs b/src/state.rs index dbd592e..d073f32 100644 --- a/src/state.rs +++ b/src/state.rs @@ -45,12 +45,18 @@ pub struct ComposeOverlay { /// hooks) and must not block every other ecluse command. A `Pending` entry /// that never transitions back means the operation crashed; `ecluse down /// ` cleans it up. +/// +/// `Stopped` means `ecluse down --keep-worktree` completed: services are +/// down and the worktree is on disk. The entry stays in state so that the +/// next `ecluse up` from inside that worktree resumes at the same slot +/// instead of allocating a new one (which would change all ports). #[derive(Debug, Clone, Copy, PartialEq, Deserialize, Serialize, Default)] #[serde(rename_all = "lowercase")] pub enum SessionStatus { #[default] Active, Pending, + Stopped, } fn is_active(status: &SessionStatus) -> bool { @@ -70,11 +76,18 @@ pub struct PendingOp { } /// Fresh operation id: unique enough to distinguish two concurrent commands. +/// +/// `pid` disambiguates across processes; the process-local counter guarantees +/// two ids minted in the same process (even within one nanosecond) never +/// collide, and the timestamp keeps ids human-readable / ordered. pub fn new_op_id() -> String { + use std::sync::atomic::{AtomicU64, Ordering}; + static COUNTER: AtomicU64 = AtomicU64::new(0); format!( - "{}-{}", + "{}-{}-{}", std::process::id(), - chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default() + chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default(), + COUNTER.fetch_add(1, Ordering::Relaxed) ) } @@ -85,8 +98,11 @@ pub struct Session { pub slot: u8, pub branch: String, pub worktree_path: String, - /// Active, or Pending while an up/down is in flight. Defaults to Active - /// so state.json files written by older versions load unchanged. + /// `Active` during normal operation; `Pending` while an up/down is in + /// flight; `Stopped` after `ecluse down --keep-worktree` (slot reserved + /// until the worktree is revived via `ecluse up`). Defaults to Active and + /// is omitted from JSON when Active, so state.json files written by older + /// versions load unchanged and stay byte-compatible. #[serde(default, skip_serializing_if = "is_active")] pub status: SessionStatus, /// Present iff status == Pending: identifies the operation that owns this @@ -145,6 +161,10 @@ impl State { self.sessions.iter().find(|s| s.slug == slug) } + pub fn find_session_mut(&mut self, slug: &str) -> Option<&mut Session> { + self.sessions.iter_mut().find(|s| s.slug == slug) + } + pub fn add_session(&mut self, session: Session) { self.sessions.push(session); } @@ -177,6 +197,36 @@ impl State { Some((original, op_id)) } + /// Transition `slug` to `Stopped`, clearing all service runtime and + /// provisioning state so a later `ecluse up` from inside the kept worktree + /// resumes at the same slot with freshly re-probed ports and overlays. Only + /// the identity fields (slug, slot, branch, worktree_path, mode) survive. + /// Returns an error if the slug is absent — callers reach this only after + /// `still_owned` confirmed the entry, so a missing slug is a broken + /// invariant that must surface loudly rather than silently skip the update. + pub fn mark_stopped(&mut self, slug: &str) -> Result<()> { + let s = self.find_session_mut(slug).ok_or_else(|| { + anyhow::anyhow!("mark_stopped: session '{slug}' not found (state invariant broken)") + })?; + s.status = SessionStatus::Stopped; + s.pending_op = None; + // Native runtime handles — services are no longer running. + s.process_manager = None; + s.tmux_session = None; + s.pid_files.clear(); + s.log_dir = None; + s.port_overrides.clear(); + s.app_port = None; + // Provisioning artifacts — the overlays were removed at teardown and are + // regenerated on the next `up`; leaving stale paths here is misleading. + s.compose_project = None; + s.overlay_file = None; + s.overlay_files.clear(); + s.compose_overlays.clear(); + s.services_subset = None; + Ok(()) + } + /// True while the Pending entry written under `op_id` is still in place — /// i.e. no other command removed or took over the session in the meantime. pub fn still_owned(&self, slug: &str, op_id: &str) -> bool { @@ -703,6 +753,27 @@ mod tests { assert!(state.still_owned("busy", &op_id)); } + // mark_pending mutates the entry in place — it does NOT remove it. A restore + // path must therefore remove-then-add, or it duplicates the session. This + // pins that invariant so the teardown-failure restore can't silently regress. + #[test] + fn mark_pending_keeps_a_single_entry() { + let mut state = State::default(); + state.add_session(make_session("busy", 1)); + state.mark_pending("busy").unwrap(); + assert_eq!( + state.sessions.iter().filter(|s| s.slug == "busy").count(), + 1 + ); + // Restore pattern: remove the Pending entry, then re-add — still one. + let original = state.remove_session("busy").unwrap(); + state.add_session(original); + assert_eq!( + state.sessions.iter().filter(|s| s.slug == "busy").count(), + 1 + ); + } + #[test] fn mark_pending_missing_session_returns_none() { let mut state = State::default(); @@ -754,4 +825,93 @@ mod tests { state.add_session(s); assert!(state.used_slots().contains(&3)); } + + // ── Stopped status ──────────────────────────────────────────────────────── + + #[test] + fn stopped_status_serializes_and_deserializes() { + let mut s = make_session("kept", 3); + s.status = SessionStatus::Stopped; + let json = serde_json::to_string(&s).unwrap(); + assert!(json.contains("\"status\":\"stopped\""), "got: {json}"); + let back: Session = serde_json::from_str(&json).unwrap(); + assert_eq!(back.status, SessionStatus::Stopped); + } + + #[test] + fn stopped_slot_is_counted_as_used() { + let mut state = State::default(); + let mut s = make_session("kept", 3); + s.status = SessionStatus::Stopped; + state.add_session(s); + assert!(state.used_slots().contains(&3)); + } + + #[test] + fn find_session_mut_updates_status() { + let mut state = State::default(); + state.add_session(make_session("x", 1)); + state.find_session_mut("x").unwrap().status = SessionStatus::Stopped; + assert_eq!( + state.find_session("x").unwrap().status, + SessionStatus::Stopped + ); + } + + #[test] + fn mark_stopped_transitions_and_clears_runtime_state() { + let mut state = State::default(); + let mut s = make_session("kept", 2); + s.status = SessionStatus::Pending; + s.pending_op = Some(PendingOp { + id: "op".into(), + since: "now".into(), + }); + s.tmux_session = Some("sess".into()); + s.pid_files = vec![PathBuf::from("/tmp/pid")]; + s.log_dir = Some(PathBuf::from("/tmp/log")); + s.port_overrides.insert("web".into(), 3002); + s.app_port = Some(3002); + s.process_manager = Some(ProcessManager::Tmux); + s.compose_project = Some("proj".into()); + s.overlay_file = Some("overlay.yml".into()); + s.overlay_files = vec!["a.yml".into()]; + s.compose_overlays = vec![ComposeOverlay { + compose: "docker-compose.yml".into(), + overlay: "overlay.yml".into(), + }]; + s.services_subset = Some(vec!["web".into()]); + state.add_session(s); + + state.mark_stopped("kept").unwrap(); + + let back = state.find_session("kept").unwrap(); + assert_eq!(back.status, SessionStatus::Stopped); + assert!(back.pending_op.is_none()); + assert!(back.tmux_session.is_none()); + assert!(back.pid_files.is_empty()); + assert!(back.log_dir.is_none()); + assert!(back.port_overrides.is_empty()); + assert!(back.app_port.is_none()); + assert!(back.process_manager.is_none()); + assert!(back.compose_project.is_none()); + assert!(back.overlay_file.is_none()); + assert!(back.overlay_files.is_empty()); + assert!(back.compose_overlays.is_empty()); + assert!(back.services_subset.is_none()); + // Identity fields must survive so `ecluse up` resumes the same worktree. + assert_eq!(back.slug, "kept"); + assert_eq!(back.slot, 2); + assert_eq!(back.worktree_path, "/tmp/kept"); + assert_eq!(back.branch, "branch/kept"); + assert_eq!(back.mode, Mode::Host); + // Slot stays reserved so `ecluse up` resumes at the same slot. + assert!(state.used_slots().contains(&2)); + } + + #[test] + fn mark_stopped_missing_session_errors() { + let mut state = State::default(); + assert!(state.mark_stopped("ghost").is_err()); + } } diff --git a/tests/integration.rs b/tests/integration.rs index 4e1cc15..6d727a4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -305,6 +305,167 @@ fn down_removes_worktree_and_clears_state() { assert!(stdout(&ls).contains("no active sessions")); } +fn write_service_config(dir: &std::path::Path) { + std::fs::write( + dir.join(".ecluse.toml"), + r#"mode = "host" + +[[services]] +name = "api" +base_port = 4000 +command = "echo api" +"#, + ) + .unwrap(); +} + +fn read_state(dir: &std::path::Path) -> serde_json::Value { + serde_json::from_str(&std::fs::read_to_string(dir.join(".ecluse/state.json")).unwrap()).unwrap() +} + +#[test] +fn down_keep_worktree_marks_stopped_and_reserves_slot() { + // The crux of the slot-preservation feature: down --keep-worktree keeps the + // session in state as Stopped (not removed), and a plain `ecluse up` + // auto-detects it, resumes at the SAME slot, and re-populates the port + // mapping that mark_stopped cleared. + let repo = tmp_repo(); + write_service_config(repo.path()); + + let up = ecluse(repo.path(), &["up", "feat-foo"]); + assert!(up.status.success(), "up failed: {}", stderr(&up)); + assert_eq!( + read_state(repo.path())["sessions"][0]["slot"].as_u64(), + Some(1) + ); + + // Tear down services but keep the worktree. + let down = ecluse(repo.path(), &["down", "feat-foo", "--keep-worktree"]); + assert!(down.status.success(), "down failed: {}", stderr(&down)); + + // State entry survives as Stopped, slot still reserved, worktree on disk. + let state = read_state(repo.path()); + assert_eq!(state["sessions"].as_array().unwrap().len(), 1); + assert_eq!(state["sessions"][0]["status"].as_str(), Some("stopped")); + assert_eq!(state["sessions"][0]["slot"].as_u64(), Some(1)); + assert!(repo.path().join(".ecluse/worktrees/feat-foo").exists()); + assert!( + stdout(&ecluse(repo.path(), &["ls"])).contains("(stopped)"), + "ls should mark the session (stopped)" + ); + + // While stopped, env errors with a pointer to `ecluse up` (no stale ports). + let env = ecluse(repo.path(), &["env", "feat-foo"]); + assert!(!env.status.success()); + assert!( + stderr(&env).contains("stopped") && stderr(&env).contains("ecluse up"), + "env on a stopped session must error with an up hint, got: {}", + stderr(&env) + ); + + // Plain `ecluse up` resumes the stopped session at the same slot, and the + // port mapping is re-probed back into state (mark_stopped had cleared it). + let reup = ecluse(repo.path(), &["up", "feat-foo"]); + assert!(reup.status.success(), "resume failed: {}", stderr(&reup)); + let state = read_state(repo.path()); + assert_eq!(state["sessions"].as_array().unwrap().len(), 1); + // Active sessions omit the status field entirely (serde skip_if is_active). + assert!(state["sessions"][0]["status"].is_null()); + assert_eq!(state["sessions"][0]["slot"].as_u64(), Some(1)); + assert_eq!( + state["sessions"][0]["port_overrides"]["api"].as_u64(), + Some(4001), + "resume must re-probe and re-record ports, got: {}", + state["sessions"][0] + ); +} + +#[test] +fn down_delete_worktree_on_stopped_session_does_not_orphan() { + // Regression: skipping bring_down for a Stopped session must still remove the + // worktree when the user asks to delete it, or `down --delete-worktree` would + // leave an orphaned directory with no state entry tracking it. + let repo = tmp_repo(); + write_service_config(repo.path()); + ecluse(repo.path(), &["up", "feat-foo"]); + ecluse(repo.path(), &["down", "feat-foo", "--keep-worktree"]); + let worktree = repo.path().join(".ecluse/worktrees/feat-foo"); + assert!(worktree.exists(), "precondition: worktree kept"); + + // Now delete the (already Stopped) session's worktree. + let out = ecluse(repo.path(), &["down", "feat-foo", "--delete-worktree"]); + assert!(out.status.success(), "{}", stderr(&out)); + assert!(!worktree.exists(), "worktree must not be orphaned on disk"); + // Directly pin the invariant: the entry is dropped, not re-marked Stopped. + assert_eq!( + read_state(repo.path())["sessions"] + .as_array() + .unwrap() + .len(), + 0 + ); + assert!(stdout(&ecluse(repo.path(), &["ls"])).contains("no active sessions")); +} + +#[test] +fn shutdown_keep_worktrees_marks_all_stopped_and_reserves_slots() { + // shutdown --keep-worktrees mirrors down --keep-worktree across every + // session: each survives in state as Stopped with its slot reserved and + // worktree on disk, and the summary calls out how many were kept. + let repo = tmp_repo(); + write_service_config(repo.path()); + ecluse(repo.path(), &["up", "feat-a"]); + ecluse(repo.path(), &["up", "feat-b"]); + + let out = ecluse(repo.path(), &["shutdown", "--keep-worktrees"]); + assert!(out.status.success(), "{}", stderr(&out)); + assert!( + stdout(&out).contains("kept as stopped"), + "summary should note kept-as-stopped sessions, got: {}", + stdout(&out) + ); + + let state = read_state(repo.path()); + let sessions = state["sessions"].as_array().unwrap(); + assert_eq!(sessions.len(), 2, "both entries must survive"); + for s in sessions { + assert_eq!(s["status"].as_str(), Some("stopped")); + } + let mut slots: Vec = sessions + .iter() + .map(|s| s["slot"].as_u64().unwrap()) + .collect(); + slots.sort_unstable(); + assert_eq!(slots, vec![1, 2], "slots stay reserved"); + assert!(repo.path().join(".ecluse/worktrees/feat-a").exists()); + assert!(repo.path().join(".ecluse/worktrees/feat-b").exists()); +} + +#[test] +fn resume_stopped_with_missing_worktree_gives_actionable_error() { + // If a stopped session's kept worktree is deleted by hand, a bare + // `ecluse up ` must not surface the generic "remove --reuse-worktree" + // advice (the user passed no such flag); it must tell them how to recover. + let repo = tmp_repo(); + write_service_config(repo.path()); + ecluse(repo.path(), &["up", "feat-foo"]); + ecluse(repo.path(), &["down", "feat-foo", "--keep-worktree"]); + let worktree = repo.path().join(".ecluse/worktrees/feat-foo"); + std::fs::remove_dir_all(&worktree).unwrap(); + + let out = ecluse(repo.path(), &["up", "feat-foo"]); + assert!(!out.status.success()); + let err = stderr(&out); + assert!( + err.contains("stopped session") && err.contains("ecluse down feat-foo"), + "error should point at recovery, not --reuse-worktree; got: {err}" + ); + assert!( + !err.contains("--reuse-worktree"), + "must not mention a flag the user never passed; got: {err}" + ); +} + #[test] fn down_nonexistent_slug_errors() { let repo = tmp_repo(); @@ -669,13 +830,21 @@ post_up = "sleep 3" .spawn() .unwrap(); - // Wait for the pending reservation, then take the session over with down. + // Wait for the pending reservation AND the worktree to exist before taking + // the session over. `mark_pending` commits the state entry before the + // worktree is created; waiting only on the entry lets `down --delete-worktree` + // race ahead of worktree creation and fail teardown (flaky). The post_up + // sleep guarantees the session is still mid-provisioning once both appear. let state_path = repo.path().join(".ecluse/state.json"); + let worktree = repo.path().join(".ecluse/worktrees/race-sess"); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); - while !std::fs::read_to_string(&state_path) - .unwrap_or_default() - .contains("race-sess") - { + loop { + let entry_present = std::fs::read_to_string(&state_path) + .unwrap_or_default() + .contains("race-sess"); + if entry_present && worktree.exists() { + break; + } assert!(std::time::Instant::now() < deadline); std::thread::sleep(std::time::Duration::from_millis(50)); }