diff --git a/CHANGELOG.md b/CHANGELOG.md index bc12c87..5eb4665 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Added +- On-demand port discovery. `ecluse ls` gains a `LISTENING` column showing the ports each session's process trees are actually bound to (trailing `!` when an assigned port isn't among them), and `ecluse status` gains an `ACTUAL` column beside `EXPECTED` listing what the service's own process tree is listening on. A service alive on the wrong port now reads `✗ wrong port 4020 (slot 2)` instead of a bare `✗ down`, and when the discovered port falls inside another slot's territory the warning names the owning slot and session and forbids killing it — pointing at `down --keep-worktree` + `up` instead. That missing attribution is what let three agents mistake each other's services for their own stale leftovers in the 2026-06-09 cross-agent kill spiral. Discovery is read-only: `state.json` stays the source of truth and a discovered port is never written back over the assigned one. Only a *missing* assigned port of a natively spawned service counts as a mismatch, so the extra sockets a dev server opens (HMR, debug, inspector) don't flag one, and neither do the Docker-held ports of hybrid sessions. `status` blames a port only on the tmux window or pid file of the service that holds it, and ignores pid files whose pid was recycled. Runs when a command is invoked — no background daemon — via a single snapshot of all listening sockets plus the process table (one `lsof` and one `ps`, independent of session count; identifying each session's processes still adds a few cheap pid-file and tmux lookups). If `lsof` is missing, discovery is disabled with a one-time warning. `--json` gains `listening_ports`/`port_mismatch` on `ls` sessions and `actual_port`/`listening_ports`/`port_mismatch`/`conflicting_slot`/`hint` on `status` services; a mismatch trips the existing non-zero exit so `ecluse status --quiet` works as a gate. (#34) + ### Changed - `ecluse down --keep-worktree` and `shutdown --keep-worktrees` now mark the session `Stopped` instead of dropping it from state. The slot stays reserved, so the next `ecluse up` resumes at the same slot and ports. (#32) - `ecluse up `, or a bare `ecluse up` from inside the kept worktree, detects a stopped session and resumes it, re-probing for free ports. `--reuse-worktree` is no longer needed. diff --git a/README.md b/README.md index 1f04215..edeb22f 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,24 @@ ecluse env feat-foo # full JSON: worktree_path, slot, all ECLUSE_* vars ecluse env # auto-detects session if run from inside a worktree ``` +**Port discovery** — `ecluse ls` and `ecluse status` also report the port each service is +*actually* listening on next to the one ecluse assigned, so a service that bound the wrong +port shows up as wrong rather than merely down. Discovery runs on invocation; there's no daemon. + +``` +$ ecluse status feat-a +SERVICE TYPE EXPECTED ACTUAL STATUS +api native 4010 4020 ✗ wrong port 4020 (slot 2) + +warning: service 'api' is listening on 4020 but ecluse assigned 4010; 4020 belongs +to slot 2 (session 'feat-b') — do not kill it, run: ecluse down feat-a +--keep-worktree && ecluse up feat-a +``` + +Assignment stays the source of truth — a discovered port is reported, never written back +over it. Under parallel sessions the process on a neighbouring port is almost always +another agent's working service, so the remedy is `down --keep-worktree` + `up`, not `kill`. + **Branch names as argument** — pass your git branch name directly; ecluse sanitizes it to a valid slug and uses the original as the branch: ```bash diff --git a/docs/src/commands.md b/docs/src/commands.md index 1f8739a..f395363 100644 --- a/docs/src/commands.md +++ b/docs/src/commands.md @@ -150,6 +150,17 @@ Lists active sessions. Use `--json` for machine-readable output. The table shows all allocated ports in a `PORTS` column as `name=value` pairs (e.g. `api=4445 postgres=5433 redis=6380`). A `TMUX` column appears when at least one session uses tmux — the value is the session name you can pass to `tmux attach -t ` or `ecluse shell `. +A `LISTENING` column shows the ports each session's processes are *actually* bound to, discovered at invocation time (no background daemon). A trailing `!` means an assigned port is not being listened on — usually a service that bound the wrong port. Run `ecluse status ` for the per-service breakdown: + +``` +SLUG MODE SLOT PORTS LISTENING BRANCH +feat-a host 1 api=4010 4020 ! feat-a +feat-b host 2 api=4020 - feat-b +feat-c host 3 api=4030 4030 24678 feat-c +``` + +Here feat-a's api bound slot 2's port, so feat-b's api could not start and nothing in feat-b is listening (`-`). feat-c is healthy: the extra HMR socket appears in `LISTENING` but does not trigger the `!`. Only a missing assigned port does, and only for native services ecluse spawns; the Docker service ports of a hybrid session are held by the Docker daemon and are not compared. `--json` adds `listening_ports` and `port_mismatch` per session. + ## ecluse validate Validates port ranges in `.ecluse.toml` and checks for gaps or collisions. Use `--ports` to preview the full port allocation table across all slots. Also checks that the configured `process_manager` binary is installed (e.g. tmux or nohup). @@ -171,6 +182,21 @@ ecluse status # auto-detect slug from cwd (must be inside a wo For native services, ecluse matches running processes in the worktree by their command line. For docker services, it queries `docker ps` by container name. +The `EXPECTED` column is the port ecluse allocated; `ACTUAL` lists the ports the service's own process tree (its pid file, or its own tmux window) is really listening on, shown only when that is something other than exactly the expected port. Extra sockets such as an HMR port are listed without marking the service wrong; `✗ wrong port` appears only when the expected port is not held by the service's tree and the tree listens on something else. A service alive on the wrong port reads `✗ wrong port ` rather than a bare `✗ down`, and when that port falls inside another slot's territory the warning names the owning slot and session: + +``` +SERVICE TYPE EXPECTED ACTUAL STATUS +api native 4010 4020 ✗ wrong port 4020 (slot 2) + +warning: service 'api' is listening on 4020 but ecluse assigned 4010; 4020 belongs +to slot 2 (session 'feat-b') — do not kill it, run: ecluse down feat-a +--keep-worktree && ecluse up feat-a +``` + +Discovery is read-only: `state.json` stays the source of truth and a discovered port is never written back over the assigned one. A mismatch means something bound the wrong port (usually an external task runner that read `.env.local` instead of `.env.ecluse`) — the fix is `down --keep-worktree` + `up`, never `kill`. `--json` adds `actual_port`, `port_mismatch`, `conflicting_slot`, and `hint` per service. + +Docker services are excluded from process-tree discovery: the daemon publishes their ports, so `docker ps` already reports the real mapping. + The last column and the session header adapt to the process manager: - **tmux** — header shows the tmux session name (`tmux=ecluse-`); last column is `WINDOW` showing the tmux window name for each native service. Health is verified by checking that a process in the pane's subtree owns the expected port — a port collision with an unrelated process correctly shows the service as down. diff --git a/docs/src/ports.md b/docs/src/ports.md index b6545ce..d67d1f8 100644 --- a/docs/src/ports.md +++ b/docs/src/ports.md @@ -80,6 +80,28 @@ Pin a specific service to a port for a session (useful when the auto-assigned po ecluse up feat-foo --port api=4001 --port postgres=5444 ``` +## Discovery: what is actually listening + +Assignment answers "which port should this service use". Discovery answers "which port is it really on". `ecluse ls` and `ecluse status` report both, so a service that bound the wrong port is visible instead of just looking down. + +Discovery runs when you invoke a command — there is no background daemon. It takes one snapshot of every listening TCP socket plus the process table (one `lsof` and one `ps`, regardless of how many sessions exist), then attributes each listener to the session whose process tree owns it. Finding each session's root processes adds a few cheap calls of its own (a liveness and start-time check per pid file, and a `tmux list-panes` for tmux sessions). If `lsof` isn't installed, discovery is disabled and ecluse warns once. + +``` +$ ecluse status feat-a +SERVICE TYPE EXPECTED ACTUAL STATUS +api native 4010 4020 ✗ wrong port 4020 (slot 2) +``` + +Because ports are derived from the slot, the formula inverts: a discovered port can be mapped back to the slot that owns it. When the wrong port belongs to another slot, ecluse names that slot and its session, and says explicitly not to kill it — under parallel sessions the process on a neighbouring port is almost always another agent's working service. + +**Discovery never overwrites assignment.** `state.json` remains the source of truth. A discovered port that disagrees with the assigned one is evidence of a bug — typically an external task runner (`task`, `make`, `npm run`) that re-read `.env.local` instead of `.env.ecluse` — not a better value to adopt. Trusting discovery is what once hid a wrong-slot spawn behind a green check while three agents killed each other's services. The fix for a mismatch is always: + +```bash +ecluse down --keep-worktree && ecluse up +``` + +Only a *missing* assigned port counts as a mismatch, and only for native services that ecluse spawns (those with a `command`). The extra sockets a dev server opens (HMR, debug, inspector) show up in the discovered set without flagging anything, and the Docker service ports of a hybrid session are left out because the Docker daemon holds them, not the session's processes. + ## Known limitation 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: diff --git a/skills/ecluse/SKILL.md b/skills/ecluse/SKILL.md index 3f862e9..7575c30 100644 --- a/skills/ecluse/SKILL.md +++ b/skills/ecluse/SKILL.md @@ -568,6 +568,21 @@ Persistent conflict: change `base_port` in the relevant `[[services]]` block, or **Root cause:** an external task runner (`task`, `make`, `npm run`, `bin/dev`) was used as the service entry point instead of `command = "..."` in `.ecluse.toml`. External runners re-read `.env.local` and inherit the spawning shell's env — neither knows about `.env.ecluse`. Under parallel sessions the spawning shell can carry env from a *different* worktree's `source .env.ecluse`, so services bind to the wrong slot's ports. Agents then see "a process on a port adjacent to mine" and kill it, believing it's their own stale leftover. +**Detection:** `ecluse status` compares the port ecluse assigned against the port the service is actually listening on, and names the slot that owns the wrong one: + +``` +SERVICE TYPE EXPECTED ACTUAL STATUS +api native 4010 4020 ✗ wrong port 4020 (slot 2) + +warning: service 'api' is listening on 4020 but ecluse assigned 4010; 4020 belongs +to slot 2 (session 'feat-b') — do not kill it, run: ecluse down feat-a +--keep-worktree && ecluse up feat-a +``` + +`ecluse ls` shows the same thing per session: a `LISTENING` column with a trailing `!` when an assigned port isn't being listened on. + +**When you see this, do NOT kill anything by port.** The process on the ACTUAL port is in your own session's process tree, but it is squatting on a port that belongs to another slot (and the session named in the warning needs it back). Killing by port, or killing whatever sits on your EXPECTED port, risks hitting another agent's service. Run the recovery below — it only touches your own services and frees the other slot's port. + **Recovery (do this in each affected session):** ```bash diff --git a/src/discover.rs b/src/discover.rs new file mode 100644 index 0000000..90ecdb0 --- /dev/null +++ b/src/discover.rs @@ -0,0 +1,485 @@ +//! On-demand port discovery. +//! +//! ecluse *assigns* ports (`port = base_port + slot × stride`) and treats +//! `state.json` as truth. Discovery is the complement: it observes which ports +//! are *actually* being listened on, so a command can report expected-vs-actual +//! instead of a bare `✗ down` when something bound the wrong port. +//! +//! Discovery never rewrites state. A mismatch is evidence of a bug (usually an +//! external task runner that re-read `.env.local` instead of `.env.ecluse`), not +//! a better value to adopt — see `incidents/2026-06-09-rubbr-cross-agent-kill-spiral`, +//! where trusting a discovered port hid a wrong-slot spawn behind a green check. +//! +//! ## Why a single snapshot +//! +//! The obvious implementation runs `lsof -p ` per process and `pgrep -P` +//! per level of the tree. With 4 sessions × ~8 processes that is 30+ forks, and +//! `lsof` costs 20–400ms each. Instead the listener and process-tree data come +//! from *two* forks, regardless of session count — all listeners plus the whole +//! process table — and the tree join happens in memory. +//! +//! The snapshot does not cover everything a command does: finding each +//! session's root pids still verifies pid files (`kill -0` + a `ps` start-time +//! check per pid file) and asks tmux for pane pids. Those calls are cheap and +//! don't scale with the number of listening sockets. + +use std::collections::{HashMap, HashSet}; +use std::process::Command; +use std::sync::OnceLock; + +/// Ensures the "lsof missing" warning is printed at most once per process. +static LSOF_MISSING_WARNED: OnceLock<()> = OnceLock::new(); + +/// Ports never reported as a session's dev server. These are system/privileged +/// services that show up in every scan and are never what a worktree allocated. +const IGNORED_PORTS: &[u16] = &[22, 80, 443]; + +/// One point-in-time view of every listening port on the host plus the process +/// table needed to attribute those ports to a process tree. +/// +/// Built with two subprocess calls (`lsof` + `ps`). Cheap enough to take on +/// every command invocation; there is no background daemon. +#[derive(Debug, Clone, Default)] +pub struct PortSnapshot { + /// pid → ports that pid is listening on directly. + ports_by_pid: HashMap>, + /// port → the pid holding it (first one seen wins; a port has one owner). + pid_by_port: HashMap, + /// ppid → direct children, for descendant walks without forking `pgrep`. + children: HashMap>, +} + +/// Take a snapshot of all listening ports and the process table. +/// +/// Best-effort by design: a missing or failing `lsof`/`ps` yields an empty (or +/// partial) snapshot rather than an error, because discovery is supplemental +/// reporting — it must never fail a command that would otherwise succeed. +pub fn snapshot() -> PortSnapshot { + parse_snapshot(&raw_listeners(), &raw_process_table()) +} + +/// Run `lsof` for every listening TCP socket on the host. One fork. +fn raw_listeners() -> String { + // -F pn emits machine-readable records: a `p` line followed by one + // `n` line per socket. -n/-P skip DNS and service-name lookups, + // which is both faster and keeps the port numeric. + match Command::new("lsof") + .args(["-iTCP", "-sTCP:LISTEN", "-n", "-P", "-F", "pn"]) + .output() + { + // lsof exits non-zero when nothing matches, which is a legitimate + // empty result — take stdout either way. + Ok(o) => String::from_utf8_lossy(&o.stdout).into_owned(), + // Spawn failure means lsof isn't installed. Discovery degrades to an + // empty snapshot, but say so once instead of silently reporting + // "nothing listening" everywhere. + Err(_) => { + LSOF_MISSING_WARNED.get_or_init(|| { + tracing::warn!( + "lsof is not available; port discovery is disabled — \ + install lsof to enable it" + ); + }); + String::new() + } + } +} + +/// Read the whole process table as `pid ppid` pairs. One fork. +fn raw_process_table() -> String { + match Command::new("ps").args(["-axo", "pid=,ppid="]).output() { + Ok(o) => String::from_utf8_lossy(&o.stdout).into_owned(), + Err(_) => String::new(), + } +} + +/// Parse `lsof -F pn` output and a `ps -axo pid=,ppid=` table into a snapshot. +/// +/// Split out from `snapshot()` so the parsing is unit-testable without +/// depending on whatever happens to be listening on the test machine. +pub fn parse_snapshot(lsof_out: &str, ps_out: &str) -> PortSnapshot { + let mut ports_by_pid: HashMap> = HashMap::new(); + let mut pid_by_port: HashMap = HashMap::new(); + + // `-F pn` is stateful: a `p` line sets the owner for every `n` + // line that follows, until the next `p` line. + let mut current_pid: Option = None; + for line in lsof_out.lines() { + let Some((tag, rest)) = line.split_at_checked(1) else { + continue; + }; + match tag { + "p" => current_pid = rest.trim().parse().ok(), + "n" => { + let Some(pid) = current_pid else { continue }; + let Some(port) = parse_listen_port(rest) else { + continue; + }; + if IGNORED_PORTS.contains(&port) { + continue; + } + let entry = ports_by_pid.entry(pid).or_default(); + if !entry.contains(&port) { + entry.push(port); + } + pid_by_port.entry(port).or_insert(pid); + } + _ => {} + } + } + + for ports in ports_by_pid.values_mut() { + ports.sort_unstable(); + } + + let mut children: HashMap> = HashMap::new(); + for line in ps_out.lines() { + let mut parts = line.split_whitespace(); + let (Some(pid), Some(ppid)) = (parts.next(), parts.next()) else { + continue; + }; + let (Ok(pid), Ok(ppid)) = (pid.parse::(), ppid.parse::()) else { + continue; + }; + children.entry(ppid).or_default().push(pid); + } + + PortSnapshot { + ports_by_pid, + pid_by_port, + children, + } +} + +/// Extract the port from an lsof address field. +/// +/// Handles the shapes lsof emits for a listening socket: +/// `*:3000`, `127.0.0.1:3000`, `[::1]:3000`, `[::]:3000`. +/// Splitting on the *last* colon is what makes the bracketed IPv6 forms work. +fn parse_listen_port(addr: &str) -> Option { + let addr = addr.trim(); + // Some lsof builds append `->` peer info; a listener shouldn't have one, + // but guard anyway so a stray record can't produce a bogus port. + let addr = addr.split("->").next()?; + addr.rsplit(':').next()?.parse().ok() +} + +impl PortSnapshot { + /// Every port listened on by `root_pid` or any of its descendants. + /// + /// This is the attribution primitive: a dev server is usually a grandchild + /// of what ecluse spawned (`sh → pnpm → node → vite`), so the port belongs + /// to the tree, not to the recorded pid. + pub fn ports_for_tree(&self, root_pid: u32) -> Vec { + let mut ports: Vec = Vec::new(); + for pid in self.tree_pids(root_pid) { + if let Some(p) = self.ports_by_pid.get(&pid) { + for port in p { + if !ports.contains(port) { + ports.push(*port); + } + } + } + } + ports.sort_unstable(); + ports + } + + /// The pid holding `port`, if anything is. + pub fn listener_pid(&self, port: u16) -> Option { + self.pid_by_port.get(&port).copied() + } + + /// `root_pid` plus every transitive descendant. + /// + /// Cycle-guarded via `seen`: a `ps` snapshot taken while pids are being + /// recycled can in principle contain a ppid loop, and an unguarded walk + /// would hang. + pub fn tree_pids(&self, root_pid: u32) -> Vec { + let mut out = Vec::new(); + let mut seen = HashSet::new(); + let mut stack = vec![root_pid]; + while let Some(pid) = stack.pop() { + if !seen.insert(pid) { + continue; + } + out.push(pid); + if let Some(kids) = self.children.get(&pid) { + stack.extend(kids); + } + } + out + } + + /// True iff `descendant` is a transitive child of `ancestor`. + /// A pid is not its own descendant. + pub fn is_descendant(&self, ancestor: u32, descendant: u32) -> bool { + ancestor != descendant && self.tree_pids(ancestor).contains(&descendant) + } +} + +/// Which slot a port belongs to under the configured allocation scheme. +/// +/// `port = base_port + slot × stride` inverts to `slot = (port - base) / stride` +/// when the remainder is zero. Reporting this is what turns "a process is on a +/// port near mine" — the inference that started the 2026-06-09 kill spiral — +/// into "that port is slot 3's, owned by session X, do not kill it". +pub fn owning_slot(port: u16, base_port: u16, slot_stride: u8, max_slots: u8) -> Option { + let stride = slot_stride.max(1) as u16; + let offset = port.checked_sub(base_port)?; + if offset % stride != 0 { + return None; + } + let slot = offset / stride; + // Slot 0 is not a valid allocation (slots are 1..=max_slots), and a port + // beyond max_slots' territory belongs to nobody. + if slot == 0 || slot > max_slots as u16 { + return None; + } + Some(slot as u8) +} + +#[cfg(test)] +mod tests { + use super::*; + + // Real `lsof -F pn` output shape: a `p` line, then `f`/`n` records, with + // the same socket listed once per file descriptor. + const LSOF: &str = "\ +p591 +f8 +n*:60277 +f9 +n*:60277 +p609 +f8 +n*:7000 +f10 +n127.0.0.1:5000 +p744 +f9 +n[::1]:27017 +f10 +n[::]:27017 +"; + + // pid ppid — 700 is a child of 609, 800 a child of 700 (grandchild of 609). + const PS: &str = "\ + 1 0 + 591 1 + 609 1 + 700 609 + 800 700 + 744 1 +"; + + fn snap() -> PortSnapshot { + parse_snapshot(LSOF, PS) + } + + #[test] + fn parses_wildcard_address() { + assert_eq!(parse_listen_port("*:3000"), Some(3000)); + } + + #[test] + fn parses_ipv4_address() { + assert_eq!(parse_listen_port("127.0.0.1:5000"), Some(5000)); + } + + // Bracketed IPv6 is why we split on the last colon, not the first. + #[test] + fn parses_ipv6_loopback() { + assert_eq!(parse_listen_port("[::1]:27017"), Some(27017)); + } + + #[test] + fn parses_ipv6_wildcard() { + assert_eq!(parse_listen_port("[::]:8080"), Some(8080)); + } + + #[test] + fn rejects_non_numeric_port() { + assert_eq!(parse_listen_port("127.0.0.1:http"), None); + } + + #[test] + fn rejects_empty_address() { + assert_eq!(parse_listen_port(""), None); + } + + #[test] + fn dedupes_same_port_across_file_descriptors() { + let s = snap(); + assert_eq!(s.ports_for_tree(591), vec![60277]); + } + + #[test] + fn collects_multiple_ports_for_one_pid() { + let s = snap(); + assert_eq!(s.ports_for_tree(609), vec![5000, 7000]); + } + + #[test] + fn dedupes_ipv4_and_ipv6_of_same_port() { + let s = snap(); + assert_eq!(s.ports_for_tree(744), vec![27017]); + } + + #[test] + fn maps_port_to_listener_pid() { + let s = snap(); + assert_eq!(s.listener_pid(7000), Some(609)); + assert_eq!(s.listener_pid(60277), Some(591)); + } + + #[test] + fn listener_pid_none_for_unheld_port() { + assert_eq!(snap().listener_pid(9999), None); + } + + // The point of the tree walk: a port held by a grandchild belongs to the + // service ecluse spawned, not to some unrelated process. + #[test] + fn attributes_descendant_port_to_root() { + // 800 (grandchild of 609) holds 4000. + let s = parse_snapshot("p800\nf8\nn*:4000\n", PS); + assert_eq!(s.ports_for_tree(609), vec![4000]); + } + + #[test] + fn does_not_attribute_sibling_port_to_root() { + let s = snap(); + // 591 is a sibling of 609, not a descendant. + assert!(!s.ports_for_tree(609).contains(&60277)); + } + + #[test] + fn ignores_system_ports() { + let s = parse_snapshot("p42\nf8\nn*:22\nf9\nn*:80\nf10\nn*:443\nf11\nn*:3000\n", ""); + assert_eq!(s.ports_for_tree(42), vec![3000]); + assert_eq!(s.listener_pid(22), None); + } + + #[test] + fn tree_pids_includes_root_and_all_descendants() { + let mut pids = snap().tree_pids(609); + pids.sort_unstable(); + assert_eq!(pids, vec![609, 700, 800]); + } + + #[test] + fn tree_pids_is_just_root_when_childless() { + assert_eq!(snap().tree_pids(744), vec![744]); + } + + #[test] + fn is_descendant_walks_multiple_levels() { + let s = snap(); + assert!(s.is_descendant(609, 700)); + assert!(s.is_descendant(609, 800)); + } + + #[test] + fn is_descendant_false_for_ancestor_direction() { + assert!(!snap().is_descendant(800, 609)); + } + + #[test] + fn is_descendant_false_for_self() { + assert!(!snap().is_descendant(609, 609)); + } + + #[test] + fn unknown_pid_has_no_ports() { + assert!(snap().ports_for_tree(999_999).is_empty()); + } + + // A ppid cycle can only come from a torn `ps` read, but an unguarded + // walk would spin forever on it. + #[test] + fn tolerates_ppid_cycle() { + let s = parse_snapshot("", "10 11\n11 10\n"); + let mut pids = s.tree_pids(10); + pids.sort_unstable(); + assert_eq!(pids, vec![10, 11]); + } + + #[test] + fn empty_input_yields_empty_snapshot() { + let s = parse_snapshot("", ""); + assert!(s.ports_for_tree(1).is_empty()); + assert_eq!(s.listener_pid(3000), None); + } + + #[test] + fn malformed_lines_are_skipped() { + // An `n` record with no preceding `p` line has no owner; garbage ps + // rows are dropped rather than panicking. + let s = parse_snapshot("n*:3000\nxjunk\np50\nn*:3001\n", "notapid alsonot\n60 x\n"); + assert_eq!(s.listener_pid(3000), None); + assert_eq!(s.listener_pid(3001), Some(50)); + assert_eq!(s.tree_pids(60), vec![60]); + } + + // ── owning_slot ─────────────────────────────────────────────────────────── + + #[test] + fn owning_slot_identifies_own_slot_stride_1() { + assert_eq!(owning_slot(3001, 3000, 1, 8), Some(1)); + assert_eq!(owning_slot(3004, 3000, 1, 8), Some(4)); + } + + #[test] + fn owning_slot_identifies_slot_with_stride_10() { + assert_eq!(owning_slot(3010, 3000, 10, 8), Some(1)); + assert_eq!(owning_slot(3030, 3000, 10, 8), Some(3)); + } + + // An auto-bumped port that lands between slots belongs to no slot — this + // is the benign case that must NOT be reported as cross-slot theft. + #[test] + fn owning_slot_none_when_not_on_stride_boundary() { + assert_eq!(owning_slot(3015, 3000, 10, 8), None); + } + + #[test] + fn owning_slot_none_for_base_port_itself() { + // base_port is slot 0 — never a valid allocation. + assert_eq!(owning_slot(3000, 3000, 1, 8), None); + } + + #[test] + fn owning_slot_none_beyond_max_slots() { + assert_eq!(owning_slot(3009, 3000, 1, 8), None); + } + + #[test] + fn owning_slot_none_below_base_port() { + assert_eq!(owning_slot(2999, 3000, 1, 8), None); + } + + #[test] + fn owning_slot_handles_last_valid_slot() { + assert_eq!(owning_slot(3008, 3000, 1, 8), Some(8)); + } + + #[test] + fn owning_slot_treats_zero_stride_as_one() { + assert_eq!(owning_slot(3002, 3000, 0, 8), Some(2)); + } + + // Guards against a real-world regression: a live snapshot must find the + // port this test process is holding. + #[test] + fn live_snapshot_finds_own_listener() { + use std::net::TcpListener; + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let s = snapshot(); + // lsof may be absent in a sandbox; only assert when it produced data. + if s.listener_pid(port).is_some() { + assert!(s.ports_for_tree(std::process::id()).contains(&port)); + } + } +} diff --git a/src/main.rs b/src/main.rs index 8e484a8..7606422 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,6 +2,7 @@ mod cli; mod compose; mod config; mod detect; +mod discover; mod docker; mod env; mod error; @@ -20,6 +21,7 @@ mod worktree; use anyhow::{Context, Result}; use clap::Parser; use std::io::{self, Write}; +use std::path::Path; use tabled::{Table, Tabled}; fn main() { @@ -456,6 +458,9 @@ mod tests { tmux_window: None, listener_pid, wrong_owner, + actual_port: None, + conflicting_slot: None, + listening_ports: vec![], } } @@ -609,6 +614,338 @@ mod tests { teardown_or_skip_stopped(&s, &config, root, false, false, &log).unwrap(); assert!(!wt.exists(), "worktree must be removed when not kept"); } + + // ── status_str: wrong-port (discovery) ──────────────────────────────────── + + #[test] + fn status_str_wrong_port_shows_discovered_port() { + let mut s = svc_status(true, false, false, None); + s.actual_port = Some(3005); + assert_eq!(status_str(&s), "\u{2717} wrong port 3005"); + } + + #[test] + fn status_str_wrong_port_names_conflicting_slot() { + let mut s = svc_status(true, false, false, None); + s.actual_port = Some(3003); + s.conflicting_slot = Some((3, Some("feat-x".into()))); + assert_eq!(status_str(&s), "\u{2717} wrong port 3003 (slot 3)"); + } + + // wrong_owner is a distinct condition (someone ELSE holds our port) and + // must keep precedence over "we are on the wrong port". + #[test] + fn status_str_wrong_owner_takes_precedence_over_wrong_port() { + let mut s = svc_status(true, false, true, Some(777)); + s.actual_port = Some(3005); + assert_eq!(status_str(&s), "\u{2717} wrong owner (PID 777)"); + } + + #[test] + fn status_str_unmanaged_ignores_wrong_port() { + let mut s = svc_status(false, false, false, None); + s.actual_port = Some(3005); + assert_eq!(status_str(&s), "\u{2014}"); + } + + // ── actual_str ──────────────────────────────────────────────────────────── + + #[test] + fn actual_str_is_dash_when_nothing_listening() { + let s = svc_status(true, false, false, None); + assert_eq!(actual_str(&s), "\u{2014}"); + } + + #[test] + fn actual_str_is_dash_when_only_assigned_port_listening() { + let mut s = svc_status(true, true, false, None); + s.listening_ports = vec![3001]; + assert_eq!(actual_str(&s), "\u{2014}"); + } + + #[test] + fn actual_str_shows_port_on_mismatch() { + let mut s = svc_status(true, false, false, None); + s.actual_port = Some(3005); + s.listening_ports = vec![3005]; + assert_eq!(actual_str(&s), "3005"); + } + + // A healthy service with an HMR socket lists both ports, but status_str + // stays "up" because actual_port (the wrong-port signal) is unset. + #[test] + fn actual_str_lists_extra_sockets_without_flagging() { + let mut s = svc_status(true, true, false, None); + s.listening_ports = vec![3001, 24678]; + assert_eq!(actual_str(&s), "3001 24678"); + assert_eq!(status_str(&s), "\u{2713} up"); + } + + // ── mismatch_hint ───────────────────────────────────────────────────────── + + #[test] + fn mismatch_hint_none_without_discovered_port() { + let s = svc_status(true, false, false, None); + assert!(mismatch_hint("feat-a", &s).is_none()); + } + + #[test] + fn mismatch_hint_points_at_down_up_not_kill() { + let mut s = svc_status(true, false, false, None); + s.actual_port = Some(3005); + let hint = mismatch_hint("feat-a", &s).unwrap(); + assert!(hint.contains("ecluse down feat-a --keep-worktree")); + assert!(hint.contains("ecluse up feat-a")); + assert!(!hint.contains("kill")); + } + + // The single most important message in the feature: an agent that reads + // this must not kill the sibling session's process. + #[test] + fn mismatch_hint_names_owning_session_and_forbids_kill() { + let mut s = svc_status(true, false, false, None); + s.actual_port = Some(3003); + s.conflicting_slot = Some((3, Some("feat-x".into()))); + let hint = mismatch_hint("feat-a", &s).unwrap(); + assert!(hint.contains("slot 3")); + assert!(hint.contains("feat-x")); + assert!(hint.contains("do not kill")); + } + + #[test] + fn mismatch_hint_reports_unowned_slot_territory() { + let mut s = svc_status(true, false, false, None); + s.actual_port = Some(3004); + s.conflicting_slot = Some((4, None)); + let hint = mismatch_hint("feat-a", &s).unwrap(); + assert!(hint.contains("slot 4's territory")); + } + + // ── listening_summary ───────────────────────────────────────────────────── + + #[test] + fn listening_summary_dash_when_nothing_listening() { + assert_eq!(listening_summary(&[3001], &[]), "-"); + } + + #[test] + fn listening_summary_no_marker_when_all_expected_present() { + assert_eq!(listening_summary(&[3001, 5433], &[3001, 5433]), "3001 5433"); + } + + #[test] + fn listening_summary_marks_missing_expected_port() { + assert_eq!(listening_summary(&[3001], &[3005]), "3005 !"); + } + + // Extra sockets (HMR, debug, inspector) are normal and must not be + // reported as a mismatch — only a MISSING assigned port is. + #[test] + fn listening_summary_tolerates_extra_ports() { + assert_eq!( + listening_summary(&[3001], &[3001, 24678]), + "3001 24678", + "extra HMR socket must not flag a mismatch" + ); + } + + #[test] + fn listening_summary_marks_partial_match() { + assert_eq!(listening_summary(&[3001, 5433], &[3001]), "3001 !"); + } + + // ── expected_native_ports / port_mismatch ───────────────────────────────── + + fn hybrid_config() -> config::Config { + toml::from_str( + r#" +mode = "hybrid" + +[[services]] +name = "api" +run = "native" +base_port = 3000 +command = "pnpm dev" + +[[services]] +name = "worker" +run = "native" +base_port = 9000 + +[[services]] +name = "postgres" +run = "docker" +base_port = 5432 +"#, + ) + .unwrap() + } + + fn session_with_ports(ports: &[(&str, u16)]) -> state::Session { + let overrides: std::collections::HashMap = + ports.iter().map(|(k, v)| (k.to_string(), *v)).collect(); + serde_json::from_value(serde_json::json!({ + "slug": "feat-a", + "mode": "hybrid", + "slot": 1, + "branch": "feat-a", + "worktree_path": "/tmp/feat-a", + "compose_project": null, + "overlay_file": null, + "app_port": null, + "started_at": "2026-01-01T00:00:00Z", + "port_overrides": overrides, + })) + .unwrap() + } + + // Hybrid sessions keep Docker service ports in port_overrides. Those are + // held by the Docker daemon, never by the session's process tree, so they + // must not count as "expected" — otherwise every hybrid session shows `!`. + #[test] + fn expected_native_ports_excludes_docker_services_in_hybrid() { + let config = hybrid_config(); + let session = session_with_ports(&[("api", 3001), ("worker", 9001), ("postgres", 5433)]); + let expected = expected_native_ports(&config, &session); + assert_eq!(expected, vec![3001], "only spawned native services count"); + assert!(!port_mismatch(&expected, &[3001])); + assert_eq!(listening_summary(&expected, &[3001]), "3001"); + } + + #[test] + fn expected_native_ports_falls_back_to_nominal_port() { + let config = hybrid_config(); + let session = session_with_ports(&[("postgres", 5433)]); + assert_eq!(expected_native_ports(&config, &session), vec![3001]); + } + + #[test] + fn port_mismatch_false_when_nothing_listening() { + assert!(!port_mismatch(&[3001], &[])); + } + + #[test] + fn port_mismatch_true_when_assigned_port_missing() { + assert!(port_mismatch(&[3001], &[3005])); + } + + #[test] + fn port_mismatch_ignores_extra_ports() { + assert!(!port_mismatch(&[3001], &[3001, 24678])); + } + + // ── discover_service ────────────────────────────────────────────────────── + + // Two tmux panes: 100 (service A, nothing listening) and 200 (service B, + // whose child 201 listens on 3002). 999 is unrelated and holds 3001. + const PS_TWO_PANES: &str = "100 1\n200 1\n201 200\n999 1\n"; + + #[test] + fn discover_service_does_not_blame_sibling_pane_port() { + let snap = discover::parse_snapshot("p201\nn*:3002\n", PS_TWO_PANES); + let a = discover_service(&snap, Some(100), true, Some(3001)); + assert!(!a.healthy); + assert!(a.tree_ports.is_empty()); + assert_eq!(a.wrong_port, None, "B's port must not be reported for A"); + let b = discover_service(&snap, Some(200), true, Some(3002)); + assert!(b.healthy); + assert_eq!(b.wrong_port, None); + assert!(!b.wrong_owner); + } + + #[test] + fn discover_service_reports_wrong_port_when_assigned_missing() { + let snap = discover::parse_snapshot("p201\nn*:3005\n", PS_TWO_PANES); + let d = discover_service(&snap, Some(200), true, Some(3002)); + assert!(!d.healthy); + assert_eq!(d.wrong_port, Some(3005)); + assert_eq!(d.tree_ports, vec![3005]); + } + + // Extra sockets while the assigned port is held are not a wrong port — + // same rule as `ecluse ls`. + #[test] + fn discover_service_tolerates_extra_sockets() { + let snap = discover::parse_snapshot("p201\nn*:3002\nn*:24678\n", PS_TWO_PANES); + let d = discover_service(&snap, Some(200), true, Some(3002)); + assert!(d.healthy); + assert_eq!(d.wrong_port, None); + assert_eq!(d.tree_ports, vec![3002, 24678]); + } + + #[test] + fn discover_service_flags_foreign_listener_as_wrong_owner() { + let snap = discover::parse_snapshot("p999\nn*:3001\n", PS_TWO_PANES); + let d = discover_service(&snap, Some(100), true, Some(3001)); + assert!(d.wrong_owner); + assert_eq!(d.listener_pid, Some(999)); + assert!(!d.healthy); + } + + #[test] + fn discover_service_descendant_listener_is_not_wrong_owner() { + let snap = discover::parse_snapshot("p201\nn*:3002\n", PS_TWO_PANES); + let d = discover_service(&snap, Some(200), true, Some(3002)); + assert_eq!(d.listener_pid, Some(201)); + assert!(!d.wrong_owner); + } + + // A recycled pid yields no root: nothing is attributed to the service, + // but a listener on its port is still somebody else's. + #[test] + fn discover_service_without_root_attributes_nothing() { + let snap = discover::parse_snapshot("p201\nn*:3002\n", PS_TWO_PANES); + let d = discover_service(&snap, None, true, Some(3002)); + assert!(!d.healthy); + assert!(d.tree_ports.is_empty()); + assert_eq!(d.wrong_port, None); + assert!(d.wrong_owner); + } + + #[test] + fn discover_service_alive_without_port_is_healthy() { + let snap = discover::parse_snapshot("", PS_TWO_PANES); + let d = discover_service(&snap, Some(100), true, None); + assert!(d.healthy); + assert_eq!(d.listener_pid, None); + } + + #[test] + fn discover_service_skips_owner_check_without_recorded_process() { + let snap = discover::parse_snapshot("p999\nn*:3001\n", PS_TWO_PANES); + let d = discover_service(&snap, None, false, Some(3001)); + assert_eq!(d.listener_pid, None); + assert!(!d.wrong_owner); + } + + // ── service_tree_root ───────────────────────────────────────────────────── + + #[test] + fn service_tree_root_accepts_live_token_verified_pid() { + let dir = TempDir::new().unwrap(); + let pid_file = dir.path().join("api.pid"); + let me = std::process::id(); + process::write_pid_file_with_token(&pid_file, me).unwrap(); + assert_eq!(service_tree_root(&pid_file, None, "api"), Some(me)); + } + + // The pid is alive but its start token doesn't match: the pid was recycled + // by an unrelated process, so its ports must not be attributed. + #[test] + fn service_tree_root_rejects_recycled_pid() { + let dir = TempDir::new().unwrap(); + let pid_file = dir.path().join("api.pid"); + let me = std::process::id(); + std::fs::write(&pid_file, format!("{me}\nnot-the-real-start-time\n")).unwrap(); + assert_eq!(service_tree_root(&pid_file, None, "api"), None); + } + + #[test] + fn service_tree_root_none_without_pid_file_or_tmux() { + let dir = TempDir::new().unwrap(); + let pid_file = dir.path().join("api.pid"); + assert_eq!(service_tree_root(&pid_file, None, "api"), None); + } } /// Sanitize a branch name or slug into a valid ecluse slug + original branch pair. @@ -1757,6 +2094,10 @@ struct SessionRow { slot: u8, #[tabled(rename = "PORTS")] ports: String, + /// Ports actually being listened on by this session's process trees. + /// `!` marks a set that differs from PORTS — see `listening_summary`. + #[tabled(rename = "LISTENING")] + listening: String, #[tabled(rename = "TMUX")] tmux: String, #[tabled(rename = "BRANCH")] @@ -1765,8 +2106,123 @@ struct SessionRow { started: String, } +/// Root PIDs whose process trees belong to `session`: the token-verified pid +/// files written at spawn, plus tmux pane PIDs for tmux-managed sessions. +/// +/// A pid whose start token no longer matches was recycled by an unrelated +/// process; attributing its ports to this session would be a misattribution of +/// exactly the kind `whose_pid` guards against. +fn session_root_pids(root: &Path, session: &state::Session) -> Vec { + let mut pids = Vec::new(); + + let pid_dir = root.join(".ecluse").join("pids").join(&session.slug); + if let Ok(entries) = std::fs::read_dir(&pid_dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().and_then(|s| s.to_str()) != Some("pid") { + continue; + } + if let Some((pid, token)) = process::read_pid_file(&path) { + if process::pid_file_alive(pid, &token) { + pids.push(pid); + } + } + } + } + + if let Some(ref tmux_session) = session.tmux_session { + pids.extend(process::tmux_session_pane_pids(tmux_session)); + } + + pids.sort_unstable(); + pids.dedup(); + pids +} + +/// Ports this session's process trees are actually listening on. +fn discovered_ports( + root: &Path, + session: &state::Session, + snap: &discover::PortSnapshot, +) -> Vec { + let mut ports: Vec = Vec::new(); + for pid in session_root_pids(root, session) { + for port in snap.ports_for_tree(pid) { + if !ports.contains(&port) { + ports.push(port); + } + } + } + ports.sort_unstable(); + ports +} + +/// Ports a session's own process trees are expected to hold: the assigned +/// ports of the native services ecluse spawns (those with a `command`). +/// +/// `port_overrides` can't be used wholesale: in hybrid mode it also carries the +/// Docker service ports, which are held by the Docker daemon/proxy and never by +/// the session's process tree, so they would always look missing. +fn expected_native_ports(config: &config::Config, session: &state::Session) -> Vec { + let mut ports: Vec = config + .services + .iter() + .filter(|svc| svc.run == config::ServiceRun::Native && svc.command.is_some()) + .filter_map(|svc| native_expected_port(config, session, svc)) + .collect(); + ports.sort_unstable(); + ports.dedup(); + ports +} + +/// The port ecluse assigned to a native service in `session`. +/// +/// `port_overrides` is the source of truth; old state files without an entry +/// for a native service fall back to the nominal port computed from config. +fn native_expected_port( + config: &config::Config, + session: &state::Session, + svc: &config::ServiceConfig, +) -> Option { + session.port_overrides.get(&svc.name).copied().or_else(|| { + if svc.base_port == 0 { + None + } else { + Some(svc.port(session.slot, config.slot_stride)) + } + }) +} + +/// True when the session is listening on something, but not on every port +/// ecluse assigned. Extra ports beyond the assigned set are normal (HMR, debug, +/// inspector) and never count as a mismatch. +fn port_mismatch(expected: &[u16], discovered: &[u16]) -> bool { + !discovered.is_empty() && expected.iter().any(|e| !discovered.contains(e)) +} + +/// Render the LISTENING column: the discovered ports, with a trailing `!` when +/// they don't match what ecluse assigned. +/// +/// Only ports ecluse allocated participate in the comparison. A dev server that +/// also opens an HMR or debug socket would otherwise show a permanent mismatch. +fn listening_summary(expected: &[u16], discovered: &[u16]) -> String { + if discovered.is_empty() { + return "-".into(); + } + let rendered = discovered + .iter() + .map(|p| p.to_string()) + .collect::>() + .join(" "); + if port_mismatch(expected, discovered) { + format!("{} !", rendered) + } else { + rendered + } +} + fn cmd_ls(args: cli::LsArgs) -> Result<()> { - let (_, root) = config::Config::find_and_load()?; + let (config, root) = config::Config::find_and_load()?; let guard = state::StateGuard::acquire_shared(&root)?; if guard.state.sessions.is_empty() { @@ -1774,9 +2230,31 @@ fn cmd_ls(args: cli::LsArgs) -> Result<()> { return Ok(()); } + // One listener/process-table snapshot shared by every session. + let snap = discover::snapshot(); + if args.json { - let json = serde_json::to_string_pretty(&guard.state.sessions)?; - println!("{}", json); + // Sessions serialize as-is, plus the discovered view alongside the + // assigned one. Never merged into port_overrides: state stays truth. + let sessions_json: Vec = guard + .state + .sessions + .iter() + .map(|s| { + let expected = expected_native_ports(&config, s); + let listening = discovered_ports(&root, s, &snap); + let mut value = serde_json::to_value(s)?; + if let Some(obj) = value.as_object_mut() { + obj.insert("listening_ports".into(), serde_json::json!(listening)); + obj.insert( + "port_mismatch".into(), + serde_json::json!(port_mismatch(&expected, &listening)), + ); + } + Ok(value) + }) + .collect::>>()?; + println!("{}", serde_json::to_string_pretty(&sessions_json)?); return Ok(()); } @@ -1796,7 +2274,10 @@ fn cmd_ls(args: cli::LsArgs) -> Result<()> { } else { pairs.join(" ") }; + let expected = expected_native_ports(&config, s); + let listening = listening_summary(&expected, &discovered_ports(&root, s, &snap)); SessionRow { + listening, slug: match s.status { state::SessionStatus::Pending => format!("{} (pending)", s.slug), state::SessionStatus::Stopped => format!("{} (stopped)", s.slug), @@ -1827,12 +2308,15 @@ fn cmd_ls(args: cli::LsArgs) -> Result<()> { use tabled::settings::{Modify, Width}; // Truncate PORTS (col 3) to 40 chars so long port lists don't wrap the header. table.with(Modify::new(Columns::single(3)).with(Width::truncate(40).suffix("…"))); + // Same for LISTENING (col 4). + table.with(Modify::new(Columns::single(4)).with(Width::truncate(40).suffix("…"))); } if !any_tmux { use tabled::settings::object::Columns; use tabled::settings::Disable; - // TMUX is column index 4 (SLUG=0, MODE=1, SLOT=2, PORTS=3, TMUX=4) - table.with(Disable::column(Columns::single(4))); + // TMUX is column index 5 + // (SLUG=0, MODE=1, SLOT=2, PORTS=3, LISTENING=4, TMUX=5) + table.with(Disable::column(Columns::single(5))); } println!("{}", table); @@ -2504,6 +2988,149 @@ struct ServiceStatus { /// hijacking the port — `ecluse status` reports the service as down /// even though something IS responding to requests. wrong_owner: bool, + /// The port this service's process tree is *actually* listening on, when + /// it differs from the assigned `port`. Populated by discovery; never + /// written back to state — a mismatch is a bug to report, not a value to + /// adopt (see `incidents/2026-06-09-rubbr-cross-agent-kill-spiral`). + actual_port: Option, + /// Set when `actual_port` falls inside another slot's territory. Names the + /// slot and, when a session holds it, that session's slug — the one fact + /// that stops an agent from killing a sibling's service. + conflicting_slot: Option<(u8, Option)>, + /// Every port this service's own process tree is listening on, from the + /// shared snapshot. Includes extra sockets (HMR, debug, inspector). + listening_ports: Vec, +} + +/// What the shared port snapshot says about one native service. +#[derive(Debug, Default, PartialEq)] +struct ServiceDiscovery { + /// The service's tree is alive and (when a port is assigned) holds it. + healthy: bool, + /// Every port the service's tree is listening on. + tree_ports: Vec, + /// Whoever holds the assigned port, if anyone. + listener_pid: Option, + /// The assigned port is held by a process outside the service's tree. + wrong_owner: bool, + /// The tree does not hold the assigned port but listens on this one. + wrong_port: Option, +} + +/// Root pid of one native service's process tree. +/// +/// The service's pid file wins when it exists, and its pid counts only if the +/// recorded start token still matches — a recycled pid belongs to an unrelated +/// process whose ports must not be attributed to this service (same check as +/// `session_root_pids`). Legacy tmux sessions without pid files fall back to +/// the pane pid of the service's own window — never every pane in the session, +/// which would blame one service for a sibling's port. +fn service_tree_root(pid_file: &Path, tmux_session: Option<&str>, window: &str) -> Option { + if let Some((pid, token)) = process::read_pid_file(pid_file) { + return process::pid_file_alive(pid, &token).then_some(pid); + } + tmux_session.and_then(|t| sync::tmux_pane_pid(t, window)) +} + +/// Classify one native service against the shared snapshot. +/// +/// `expect_owner` is true when ecluse recorded a process for this service, so +/// a foreign listener on the assigned port means someone else holds it. +/// +/// "Wrong port" is reported only when the tree does NOT hold the assigned port +/// and does listen on something else. When the assigned port is held, extra +/// sockets are normal and are reported in `tree_ports` without flagging +/// anything — the same rule `ecluse ls` applies. +fn discover_service( + snap: &discover::PortSnapshot, + root: Option, + expect_owner: bool, + port: Option, +) -> ServiceDiscovery { + let tree_ports = root.map(|r| snap.ports_for_tree(r)).unwrap_or_default(); + let healthy = match (root, port) { + (None, _) => false, + (Some(_), None) => true, + (Some(_), Some(p)) => tree_ports.contains(&p), + }; + let listener_pid = if expect_owner { + port.and_then(|p| snap.listener_pid(p)) + } else { + None + }; + let wrong_owner = match listener_pid { + Some(actual) => root.is_none_or(|r| actual != r && !snap.is_descendant(r, actual)), + None => false, + }; + let wrong_port = match port { + Some(p) if !tree_ports.contains(&p) => tree_ports.first().copied(), + _ => None, + }; + ServiceDiscovery { + healthy, + tree_ports, + listener_pid, + wrong_owner, + wrong_port, + } +} + +/// Human-readable ACTUAL column: every port the service's tree is listening +/// on, or `—` when that is nothing or exactly the assigned port (nothing +/// interesting to report). +fn actual_str(s: &ServiceStatus) -> String { + let only_assigned = s.port.is_some_and(|p| s.listening_ports == [p]); + if s.listening_ports.is_empty() || only_assigned { + return "\u{2014}".into(); + } + s.listening_ports + .iter() + .map(|p| p.to_string()) + .collect::>() + .join(" ") +} + +/// The remediation hint for a wrong-port service. +/// +/// Always points at `down --keep-worktree` + `up`, which is idempotent, only +/// touches this session's own services, and re-probes ports. Never suggests +/// `kill`: under parallel sessions the process on a neighbouring port is +/// almost always another agent's working service. +fn mismatch_hint(slug: &str, s: &ServiceStatus) -> Option { + let actual = s.actual_port?; + let reset = format!( + "run: ecluse down {} --keep-worktree && ecluse up {}", + slug, slug + ); + Some(match &s.conflicting_slot { + Some((slot, Some(owner))) => format!( + "service '{}' is listening on {} but ecluse assigned {}; {} belongs to slot {} \ + (session '{}') — do not kill it, {}", + s.name, + actual, + s.port.map(|p| p.to_string()).unwrap_or_else(|| "-".into()), + actual, + slot, + owner, + reset + ), + Some((slot, None)) => format!( + "service '{}' is listening on {} but ecluse assigned {}; {} is slot {}'s territory — {}", + s.name, + actual, + s.port.map(|p| p.to_string()).unwrap_or_else(|| "-".into()), + actual, + slot, + reset + ), + None => format!( + "service '{}' is listening on {} but ecluse assigned {}; {}", + s.name, + actual, + s.port.map(|p| p.to_string()).unwrap_or_else(|| "-".into()), + reset + ), + }) } /// Human-readable status string for a service row. Extracted from cmd_status @@ -2519,6 +3146,14 @@ fn status_str(s: &ServiceStatus) -> String { Some(pid) => format!("\u{2717} wrong owner (PID {})", pid), None => "\u{2717} wrong owner".into(), } + } else if let Some(actual) = s.actual_port { + // Discovery found the service alive on a different port than the one + // ecluse assigned. Reporting a bare "down" here is what left agents + // guessing (and reaching for `kill`) in the 2026-06-09 incident. + match &s.conflicting_slot { + Some((slot, _)) => format!("\u{2717} wrong port {} (slot {})", actual, slot), + None => format!("\u{2717} wrong port {}", actual), + } } else if s.healthy { "\u{2713} up".into() } else { @@ -2532,8 +3167,10 @@ struct StatusRowTmux { service: String, #[tabled(rename = "TYPE")] kind: String, - #[tabled(rename = "PORT")] + #[tabled(rename = "EXPECTED")] port: String, + #[tabled(rename = "ACTUAL")] + actual: String, #[tabled(rename = "STATUS")] status: String, #[tabled(rename = "WINDOW")] @@ -2546,8 +3183,10 @@ struct StatusRowNohup { service: String, #[tabled(rename = "TYPE")] kind: String, - #[tabled(rename = "PORT")] + #[tabled(rename = "EXPECTED")] port: String, + #[tabled(rename = "ACTUAL")] + actual: String, #[tabled(rename = "STATUS")] status: String, #[tabled(rename = "PID")] @@ -2560,8 +3199,10 @@ struct StatusRowNone { service: String, #[tabled(rename = "TYPE")] kind: String, - #[tabled(rename = "PORT")] + #[tabled(rename = "EXPECTED")] port: String, + #[tabled(rename = "ACTUAL")] + actual: String, #[tabled(rename = "STATUS")] status: String, } @@ -2601,6 +3242,16 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { vec![] }; + // One snapshot shared by every service row. + let snap = discover::snapshot(); + // Slot → slug, so a cross-slot port can name the session that owns it. + let slot_owners: std::collections::HashMap = guard + .state + .sessions + .iter() + .map(|s| (s.slot, s.slug.clone())) + .collect(); + let mut statuses: Vec = Vec::new(); for svc in &native_svcs { @@ -2609,28 +3260,21 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { // if the process tree contains a listener on a different port (e.g. a // child task spawned its own server), trusting that port would make // `status` lie about what's actually wired up. - let expected_port: Option = - session.port_overrides.get(&svc.name).copied().or_else(|| { - // Fallback for old state.json files that don't have port_overrides - // for native services — compute the nominal port from config. - if svc.base_port == 0 { - None - } else { - Some(svc.port(session.slot, config.slot_stride)) - } - }); + let expected_port = native_expected_port(&config, &session, svc); // Identity first: the session's own pid file (token-verified) or tmux - // window decides health — never an lsof scan that can misattribute a - // neighbor's process. + // window decides health — never a host-wide scan that can misattribute + // a neighbor's process. `recorded_pid` is what the pid file says (shown + // in the PID column); `root` is that pid only if it is still the same + // process incarnation, so a recycled pid never has ports attributed. let pid_file = root .join(".ecluse") .join("pids") .join(&session.slug) .join(format!("{}.pid", svc.name)); let recorded_pid = process::read_pid_file(&pid_file).map(|(pid, _)| pid); - let healthy = sync::native_service_running(&root, &session, &svc.name, expected_port); - let (healthy, pid, port) = (healthy, recorded_pid, expected_port); + let tree_root = service_tree_root(&pid_file, session.tmux_session.as_deref(), &svc.name); + let (pid, port) = (recorded_pid, expected_port); let tmux_window = if matches!(session.process_manager, Some(process::ProcessManager::Tmux)) { Some(svc.name.clone()) @@ -2641,30 +3285,27 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { // ecluse — don't report them as down. let managed = svc.command.is_some(); - // Listener identity check: if SOME process is bound to the expected - // port and it's neither this service's recorded PID nor a descendant - // of it, the port is being served by an orphan from a previous - // session (or unrelated software). Surface this rather than silently - // reporting healthy=true — the service is technically alive but the - // user is hitting the wrong process. - let (listener_pid, wrong_owner) = if managed { - match (port, pid) { - (Some(p), Some(stored)) => match validate::port_listener(p) { - Some(actual) - if actual != stored - && actual != 0 - && !whose_pid::is_descendant(stored, actual) => - { - (Some(actual), true) - } - other => (other, false), - }, - _ => (None, false), - } + let disc = discover_service(&snap, tree_root, pid.is_some() || tree_root.is_some(), port); + let wrong_owner = managed && disc.wrong_owner; + let listener_pid = if managed { disc.listener_pid } else { None }; + let healthy_with_owner_check = disc.healthy && !wrong_owner; + + let (actual_port, conflicting_slot) = if managed { + let conflict = disc.wrong_port.and_then(|a| { + discover::owning_slot( + a, + svc.host_port_base(), + config.slot_stride, + config.max_slots, + ) + .filter(|slot| *slot != session.slot) + .map(|slot| (slot, slot_owners.get(&slot).cloned())) + }); + (disc.wrong_port, conflict) } else { - (None, false) + (None, None) }; - let healthy_with_owner_check = healthy && !wrong_owner; + let listening_ports = if managed { disc.tree_ports } else { vec![] }; statuses.push(ServiceStatus { name: svc.name.clone(), @@ -2676,6 +3317,9 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { tmux_window, listener_pid, wrong_owner, + actual_port, + conflicting_slot, + listening_ports, }); } @@ -2696,6 +3340,12 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { tmux_window: None, listener_pid: None, wrong_owner: false, + // Docker publishes ports through the daemon, not a host process + // tree, so process-tree discovery doesn't apply. `find_docker_services` + // already reports the real published port. + actual_port: None, + conflicting_slot: None, + listening_ports: vec![], }); } @@ -2715,6 +3365,13 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { "tmux_window": s.tmux_window, "listener_pid": s.listener_pid, "wrong_owner": s.wrong_owner, + "actual_port": s.actual_port, + "listening_ports": s.listening_ports, + "port_mismatch": s.actual_port.is_some(), + "conflicting_slot": s.conflicting_slot.as_ref().map(|(slot, owner)| { + serde_json::json!({ "slot": slot, "session": owner }) + }), + "hint": mismatch_hint(&session.slug, s), }) }) .collect(); @@ -2755,6 +3412,7 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { service: s.name.clone(), kind: s.kind.to_string(), port: port_str(s), + actual: actual_str(s), status: status_str(s), window: s.tmux_window.clone().unwrap_or_else(|| "-".into()), }) @@ -2768,6 +3426,7 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { service: s.name.clone(), kind: s.kind.to_string(), port: port_str(s), + actual: actual_str(s), status: status_str(s), pid: s.pid.map(|p| p.to_string()).unwrap_or_else(|| "-".into()), }) @@ -2781,6 +3440,7 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { service: s.name.clone(), kind: s.kind.to_string(), port: port_str(s), + actual: actual_str(s), status: status_str(s), }) .collect(); @@ -2797,6 +3457,16 @@ fn cmd_status(args: cli::StatusArgs) -> Result<()> { if down_count == 1 { "" } else { "s" } ); } + + // Explain every wrong-port service and name the safe remedy. This + // is the whole point of discovery: an agent that reads "wrong + // port, run down/up" doesn't invent a theory and reach for `kill`. + let log = log::StepLogger::new(false); + for s in &statuses { + if let Some(hint) = mismatch_hint(&session.slug, s) { + log.warn(&hint); + } + } } } diff --git a/src/process.rs b/src/process.rs index 659f7f4..c855b2c 100644 --- a/src/process.rs +++ b/src/process.rs @@ -592,7 +592,7 @@ fn kill_tmux(result: &SpawnResult) { } /// All pane PIDs across all windows of `session`. Empty on any tmux failure. -fn tmux_session_pane_pids(session: &str) -> Vec { +pub fn tmux_session_pane_pids(session: &str) -> Vec { let Ok(out) = Command::new("tmux") .args(["list-panes", "-s", "-t", session, "-F", "#{pane_pid}"]) .output() @@ -1055,16 +1055,24 @@ mod tests { ) .unwrap(); + // Wait for the pid to be readable, not merely for the file to exist: + // the shell creates the file on redirect and writes to it a moment + // later, so an existence-only wait can read an empty file and panic on + // parse. Under full-suite load that window is wide enough to hit. + let read_child_pid = || -> Option { + std::fs::read_to_string(&child_pid_file) + .ok()? + .trim() + .parse() + .ok() + }; assert!( - wait_until(std::time::Duration::from_secs(5), || child_pid_file - .exists()), - "child pid file never appeared" + wait_until(std::time::Duration::from_secs(5), || read_child_pid() + .is_some()), + "child pid never appeared in {}", + child_pid_file.display() ); - let child_pid: u32 = std::fs::read_to_string(&child_pid_file) - .unwrap() - .trim() - .parse() - .unwrap(); + let child_pid = read_child_pid().expect("child pid readable after wait"); assert!(pid_alive(child_pid), "background child should be running"); kill_services(&ProcessManager::Nohup, &result); diff --git a/src/sync.rs b/src/sync.rs index bf1ca10..8830783 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -311,7 +311,7 @@ pub(crate) fn child_pids(pid: u32) -> Vec { /// /// Returns `None` if tmux is unavailable, the session doesn't exist, or the /// window doesn't exist (tmux exits non-zero in all these cases). -fn tmux_pane_pid(session: &str, window: &str) -> Option { +pub(crate) fn tmux_pane_pid(session: &str, window: &str) -> Option { let target = format!("{}:{}", session, window); let output = Command::new("tmux") .args(["list-panes", "-t", &target, "-F", "#{pane_pid}"])