node-app-build 6.12.0

//! Port ownership attribution + per-lane allocation for the harness (and,
//! by extension, any `MonorepoHost` caller — `free_port` in
//! `dev/host/monorepo.rs` is shared by plain `node-app dev` and `harness
//! up`). This is the ONLY place allowed to decide a listening process may
//! be terminated. See spec D7 / plan Task 1 (`docs/superpowers/specs/
//! 2026-08-10-harness-browser-pairing-design.md`), filed as #2221 after
//! `harness up` SIGKILLed a different session's fully-onboarded alice+bob.
//!
//! SAFETY-CRITICAL ASYMMETRY, stated once here and echoed at every decision
//! point that reads this module: classification must never GUESS "ours". A
//! wrong `Foreign` costs the operator one actionable error message; a wrong
//! `Ours` silently destroys a different session's work. Every branch below
//! therefore requires POSITIVE, exact evidence of ownership — an unknown
//! holder, unparseable `lsof`/`ps` output, or a near-miss path all fall
//! through to `Foreign`. Do not "helpfully" relax this for convenience; that
//! is exactly how the bug this module fixes came back.
//!
//! Only two signals are ever POSITIVE evidence of ownership: a PID this
//! checkout's own harness state recorded, or an EXACT match on a holder's
//! `--env <path>` argument. `PortHolder.cwd` is collected and reported for
//! humans reading an error message, never matched against — a coincidental
//! cwd (an operator's shell simply sitting inside the checkout) is not proof
//! of anything, and treating it as a signal would be exactly the kind of
//! "helpful" relaxation the paragraph above forbids.
//!
//! Residual TOCTOU: `free_port` classifies each PID, then (for an `Ours`
//! verdict) signals it moments later. A PID can theoretically be reused by
//! an unrelated process in that window. The window is milliseconds, this is
//! interactive dev tooling rather than a security boundary, and the
//! classification is re-derived (not cached) on every call — but it is a
//! real, unclosed gap and is recorded here rather than left implicit.

use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};

use super::state::HarnessState;

/// Who (if anyone) is listening on a port, and what we could learn about them.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct PortHolder {
    pub pid: u32,
    /// The holder's current working directory (`lsof -a -p <pid> -d cwd`).
    /// Empty when the lookup failed or returned nothing parseable.
    ///
    /// DIAGNOSTIC ONLY — never a matching signal. It exists so an operator
    /// reading a "refusing to free port, held by pid N" error has a second
    /// clue about what that PID is, nothing more. A process's cwd can match
    /// a checkout's directory by pure coincidence (an operator's shell
    /// sitting there, an unrelated `find`), so classification never treats
    /// it as evidence of ownership — only an exact `--env` match or a
    /// recorded PID does (see the module doc's asymmetry note).
    pub cwd: PathBuf,
    /// The `--env <path>` argument on the holder's command line, if its
    /// command line has one at all. `None` when `ps` failed, the holder
    /// isn't an `--env`-launched process, or parsing found nothing.
    pub env_path: Option<PathBuf>,
}

/// The outcome of classifying a `PortHolder` (or the absence of one).
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum PortOwnership {
    /// Nothing is listening — safe to bind.
    Free,
    /// Attributable to this invocation — safe to terminate exactly as before.
    Ours(PortHolder),
    /// Not attributable to us. NEVER terminate this process.
    Foreign(PortHolder),
}

// ─── Attribution ──────────────────────────────────────────────────────────

/// Classify a holder using ONLY the harness state for this checkout: `Ours`
/// if its PID is one a previous `harness up` for this exact checkout
/// recorded, or its `--env` argument resolves under a recorded instance's
/// own root directory (`session_path`'s grandparent — see
/// `probes::kill_recorded_daemons` for the same derivation). Everything
/// else is `Foreign`, including a holder we simply have no data on.
///
/// This is the literal "PID appears in `HarnessState.instances[].pid` OR
/// its `--env` path resolves under this invocation's directory" rule from
/// spec D7, scoped to the harness-state signal alone. `classify_port_holder`
/// layers in a second, state-independent signal (this call's own expected
/// env path) so plain `node-app dev` — which never writes a harness state
/// file — still gets ownership protection.
pub fn classify_holder(holder: PortHolder, state: &HarnessState) -> PortOwnership {
    let pid_recorded = state.instances.iter().any(|inst| inst.pid == Some(holder.pid));
    let env_under_known_instance = holder.env_path.as_deref().is_some_and(|env_path| {
        state.instances.iter().any(|inst| {
            instance_root(&inst.session_path)
                .map(|root| env_path.starts_with(&root))
                .unwrap_or(false)
        })
    });
    if pid_recorded || env_under_known_instance {
        PortOwnership::Ours(holder)
    } else {
        PortOwnership::Foreign(holder)
    }
}

/// `session_path` = `<instance-root>/dev-apps/<name>-agent-session.json`;
/// walk up two levels to the instance root (mirrors
/// `probes::kill_recorded_daemons` and `probes::logs`).
fn instance_root(session_path: &Path) -> Option<PathBuf> {
    session_path.parent().and_then(Path::parent).map(Path::to_path_buf)
}

/// The real entry point every `free_port` call site uses. Adds a second,
/// state-independent signal to `classify_holder`: `expected_env_path` is
/// THIS invocation's own `<instance-root>/daemon.env` (computed by the
/// caller via `monorepo_env_dir`, which it already has — this module has no
/// need to duplicate that hashing). A holder whose OWN `--env` argument is
/// exactly that path was launched under this literal (monorepo, instance)
/// pair and belongs to us, regardless of whether a harness state file
/// exists at all (plain `node-app dev` never writes one).
///
/// SAFETY-CRITICAL ASYMMETRY (see module doc): both checks require an EXACT
/// match. A holder we can't positively place — including one whose `--env`
/// argument is merely close, or whose lookups came back empty — is
/// `Foreign`. There is no fallback branch that treats uncertainty as
/// permission to kill.
pub fn classify_port_holder(
    holder: PortHolder,
    expected_env_path: Option<&Path>,
    state: Option<&HarnessState>,
) -> PortOwnership {
    let owns_by_env = matches!(
        (holder.env_path.as_deref(), expected_env_path),
        (Some(actual), Some(expected)) if actual == expected
    );
    let owns_by_state = state.is_some_and(|s| {
        matches!(classify_holder(holder.clone(), s), PortOwnership::Ours(_))
    });
    if owns_by_env || owns_by_state {
        PortOwnership::Ours(holder)
    } else {
        PortOwnership::Foreign(holder)
    }
}

/// Discover who (if anyone) is listening on `port`, then classify them.
/// Convenience wrapper over `discover_port_holder` + `classify_port_holder`
/// for callers that only have a single PID to worry about; `free_port`
/// itself loops over `pids_listening_on`'s Vec directly since a port can in
/// principle have more than one listener.
pub fn classify_port(
    port: u16,
    expected_env_path: Option<&Path>,
    state: Option<&HarnessState>,
) -> PortOwnership {
    match discover_port_holder(port) {
        None => PortOwnership::Free,
        Some(holder) => classify_port_holder(holder, expected_env_path, state),
    }
}

// ─── Per-lane port allocation ─────────────────────────────────────────────

/// Port offset applied to an instance's HTTP/HTTPS/UI/OTA/LDK-P2P port when
/// `--client-node` is set. Replaces `MonorepoHost::new`'s single flat
/// `CLIENT_NODE_PORT_OFFSET` (added to every instance uniformly) with a
/// per-instance value, following the `OTA_HTTP_PORT` precedent already in
/// this codebase: derive the offset from the instance's own identity rather
/// than sharing one constant across every instance (see the `OTA_HTTP_PORT`
/// comment in `dev/host/monorepo.rs`).
///
/// `alice` keeps `CLIENT_NODE_PORT_OFFSET` (http 3001 → 3301) — that literal
/// is baked into multiple docs and Playwright fixtures as the client-node
/// default origin (`localhost:3301`) and must not move. Reusing the
/// constant here (rather than a second `300` literal) is deliberate: the
/// two must never drift, so one derives from the other instead of being
/// kept in sync by hand. `bob` gets a distinct stride so the offset itself
/// can never collide, independent of whether two instances' base ports
/// happen to differ (today alice=3001/bob=3002 already differ, but a flat
/// shared offset was still the wrong invariant to encode).
///
/// This offset is only the STARTING candidate for a lane's ports —
/// `allocate_lane_ports` probes from it and shifts to the next slot when a
/// `Foreign` holder occupies it, which is what lets two SEPARATE harness
/// invocations for the SAME instance name (two checkouts both running
/// `--client-node alice`) coexist instead of one refusing to start.
pub fn lane_offset(instance: &str, client_node: bool) -> u16 {
    if !client_node {
        return 0;
    }
    match instance {
        "alice" => crate::commands::dev::host::monorepo::CLIENT_NODE_PORT_OFFSET,
        "bob" => crate::commands::dev::host::monorepo::CLIENT_NODE_PORT_OFFSET + 100,
        // Future named instances: deterministic and bounded, so the
        // function never panics or degenerates for a name it doesn't know
        // about yet. Collisions here are a UX/doc concern (two odd instance
        // names landing on the same stride) — never a safety concern, since
        // the ownership check above is what actually prevents destructive
        // collisions.
        other => 500 + (stable_hash(other) % 400),
    }
}

fn stable_hash(s: &str) -> u16 {
    use std::hash::{Hash, Hasher};
    let mut h = std::collections::hash_map::DefaultHasher::new();
    s.hash(&mut h);
    (h.finish() % (u16::MAX as u64)) as u16
}

/// The four ports one instance's client-node lane binds together. Kept as a
/// single unit (rather than four loose `u16`s) so allocation can never split
/// an instance across two lanes — e.g. picking a `Free` http port from slot 0
/// but a `Foreign` https port forces the WHOLE slot to be rejected, not just
/// the one blocked port.
#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
pub struct PortSet {
    pub http: u16,
    pub https: u16,
    pub p2p: u16,
    pub ui: u16,
}

impl PortSet {
    /// The documented base for `instance`'s client-node lane — `lane_offset`
    /// applied to each of the base `InstanceProfile`'s four ports.
    pub fn base_for(
        instance: &str,
        http: u16,
        https: u16,
        p2p: u16,
        ui: u16,
    ) -> Self {
        let offset = lane_offset(instance, true);
        Self { http: http + offset, https: https + offset, p2p: p2p + offset, ui: ui + offset }
    }

    /// Slide the whole set forward by `delta`, preserving each port's
    /// distance from the others — the same relative shape moved as one unit.
    fn shifted(self, delta: u16) -> Self {
        Self {
            http: self.http + delta,
            https: self.https + delta,
            p2p: self.p2p + delta,
            ui: self.ui + delta,
        }
    }

    fn fields(self) -> [(&'static str, u16); 4] {
        [("http", self.http), ("https", self.https), ("p2p", self.p2p), ("ui", self.ui)]
    }
}

/// Gap between candidate lane slots when the documented base is occupied.
/// Large enough that one slot's four ports (already spread out across
/// distinct literal bases in `InstanceProfile`) don't tile into the NEXT
/// slot's ports within a handful of attempts.
const LANE_SLOT_STRIDE: u16 = 50;

/// How many candidate slots `allocate_lane_ports` tries before giving up.
/// Bounded so a genuinely saturated box fails fast with a clear message
/// instead of scanning the port space forever.
const MAX_LANE_ATTEMPTS: u16 = 5;

/// Allocate a client-node lane's port set by probing from the documented
/// `base`, advancing to the next candidate slot only when the CURRENT one
/// has a `Foreign` holder on ANY of its four ports.
///
/// The first candidate IS `base`, so the common case — one harness on a box
/// — always gets the well-known literal (alice client-node = 3301, etc.)
/// unchanged; only a genuinely occupied base causes a shift. A shifted
/// result stops being predictable, which is exactly why the caller must
/// RECORD it (`harness-state.json`) rather than let any probe re-derive it
/// from `lane_offset` alone.
///
/// Reuses `classify_port_holder` — the identical ownership logic
/// `free_port` uses — so a `Foreign` holder here means "skip this slot",
/// never "kill it": this function must never terminate anything, and
/// contains no code path that could.
pub fn allocate_lane_ports(
    base: PortSet,
    expected_env_path: Option<&Path>,
    state: Option<&HarnessState>,
) -> anyhow::Result<PortSet> {
    allocate_lane_ports_with(base, |port| classify_port(port, expected_env_path, state))
}

/// `allocate_lane_ports`'s algorithm, split out from the real `lsof`/`ps`
/// probing so it can be exercised hermetically: tests inject a canned
/// `classify` instead of depending on real listening sockets (though one
/// integration test below still does, for end-to-end confidence — see
/// `allocate_lane_ports_resolves_a_second_lane_around_a_real_foreign_holder`).
fn allocate_lane_ports_with(
    base: PortSet,
    mut classify: impl FnMut(u16) -> PortOwnership,
) -> anyhow::Result<PortSet> {
    let mut tried = Vec::new();
    for attempt in 0..MAX_LANE_ATTEMPTS {
        let candidate = base.shifted(attempt * LANE_SLOT_STRIDE);
        let mut blockers = Vec::new();
        for (label, port) in candidate.fields() {
            if let PortOwnership::Foreign(h) = classify(port) {
                blockers.push(format!(
                    "{label} {port} held by pid {} (cwd {}, env {})",
                    h.pid,
                    h.cwd.display(),
                    h.env_path
                        .as_deref()
                        .map(|p| p.display().to_string())
                        .unwrap_or_else(|| "<unknown>".into()),
                ));
            }
        }
        if blockers.is_empty() {
            return Ok(candidate);
        }
        tried.push(format!("slot {attempt} (http {}): {}", candidate.http, blockers.join("; ")));
    }
    anyhow::bail!(
        "no free client-node lane found after {MAX_LANE_ATTEMPTS} candidate slot(s) starting \
         from http {}:\n{}\n\
         Every candidate was held by a process this invocation cannot attribute to itself. If \
         one of those is actually yours, stop it (`node-app harness down` for a stale harness) \
         and re-run; otherwise another session is genuinely using this box's client-node lanes \
         and there is no free slot left to allocate.",
        base.http,
        tried.join("\n"),
    );
}

// ─── `lsof`/`ps`-backed discovery ──────────────────────────────────────────

/// Find the PID listening on `port`, then describe it. Returns `None` when
/// nothing is listening OR `lsof` itself is unavailable/failed — in both
/// cases the caller proceeds as if the port were free, exactly like
/// `pids_listening_on` already does elsewhere in `dev/host/monorepo.rs`: a
/// subsequent bind failing loudly on EADDRINUSE is the safe fallback here,
/// never a silent kill.
pub fn discover_port_holder(port: u16) -> Option<PortHolder> {
    let pid = listening_pid(port)?;
    Some(describe_pid(pid))
}

fn listening_pid(port: u16) -> Option<u32> {
    let arg = format!("-iTCP:{port}");
    let output = Command::new("lsof")
        .args(["-nP", "-sTCP:LISTEN", &arg])
        .stdin(Stdio::null())
        .stdout(Stdio::piped())
        .stderr(Stdio::null())
        .output()
        .ok()?;
    if !output.status.success() {
        return None;
    }
    parse_listening_pid(&String::from_utf8_lossy(&output.stdout))
}

/// Parse the PID column out of `lsof -nP -iTCP:<port> -sTCP:LISTEN` output
/// (a header line, then one row per holder). Anything that doesn't look
/// like that listing — including empty/garbage input — yields `None` rather
/// than a guess.
fn parse_listening_pid(listing: &str) -> Option<u32> {
    listing
        .lines()
        .skip(1) // header: COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
        .find_map(|line| line.split_whitespace().nth(1).and_then(|p| p.parse::<u32>().ok()))
}

/// Describe the process holding a port: its cwd (diagnostic only — see
/// `PortHolder::cwd`) and the `--env <path>` argument on its command line
/// (the only positive attribution signal besides a recorded PID). Both
/// lookups are best-effort — a failure yields an under-specified
/// `PortHolder` that `classify_port_holder` can only ever resolve to
/// `Foreign`, never `Ours`, because ownership requires a positive, exact
/// match.
pub fn describe_pid(pid: u32) -> PortHolder {
    PortHolder { pid, cwd: pid_cwd(pid).unwrap_or_default(), env_path: pid_env_arg(pid) }
}

fn pid_cwd(pid: u32) -> Option<PathBuf> {
    // `-Fn` (field mode) instead of the default columnar table: one
    // letter-prefixed field per line, immune to path-with-spaces column
    // misalignment.
    let output = Command::new("lsof")
        .args(["-a", "-p", &pid.to_string(), "-d", "cwd", "-Fn"])
        .stdin(Stdio::null())
        .stdout(Stdio::piped())
        .stderr(Stdio::null())
        .output()
        .ok()?;
    if !output.status.success() {
        return None;
    }
    parse_lsof_cwd(&String::from_utf8_lossy(&output.stdout))
}

/// Parse `lsof -Fn` field output for the `n`-prefixed (name/path) line.
fn parse_lsof_cwd(output: &str) -> Option<PathBuf> {
    output.lines().find_map(|l| l.strip_prefix('n')).filter(|p| !p.is_empty()).map(PathBuf::from)
}

fn pid_env_arg(pid: u32) -> Option<PathBuf> {
    let output = Command::new("ps")
        .args(["-o", "command=", "-p", &pid.to_string()])
        .stdin(Stdio::null())
        .stdout(Stdio::piped())
        .stderr(Stdio::null())
        .output()
        .ok()?;
    if !output.status.success() {
        return None;
    }
    parse_env_arg(&String::from_utf8_lossy(&output.stdout))
}

/// Extract the `--env <path>` argument from a process's command line — the
/// same extraction `probes::parse_orphaned_daemons` uses to recognize a
/// harness/dev-generated daemon.
fn parse_env_arg(command: &str) -> Option<PathBuf> {
    let rest = command.split("--env ").nth(1)?;
    let env_path = rest.split_whitespace().next()?;
    if env_path.is_empty() {
        return None;
    }
    Some(PathBuf::from(env_path))
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::commands::harness::state::InstanceState;

    // ─── classify_holder (state-only signal) ───────────────────────────

    #[test]
    fn a_holder_absent_from_harness_state_is_foreign() {
        let state = HarnessState::default();
        let holder = PortHolder { pid: 4242, cwd: "/tmp/other".into(), env_path: None };
        assert!(matches!(classify_holder(holder, &state), PortOwnership::Foreign(_)));
    }

    #[test]
    fn a_holder_recorded_in_harness_state_is_ours() {
        let mut state = HarnessState::default();
        state.instances.push(InstanceState { pid: Some(4242), ..InstanceState::default() });
        let holder = PortHolder { pid: 4242, cwd: "/tmp/mine".into(), env_path: None };
        assert!(matches!(classify_holder(holder, &state), PortOwnership::Ours(_)));
    }

    #[test]
    fn env_path_under_a_known_instance_root_is_ours_even_with_a_different_pid() {
        // Covers a restarted daemon: same checkout, new PID, but its --env
        // argument still lands under this checkout's own instance root.
        let mut state = HarnessState::default();
        state.instances.push(InstanceState {
            session_path: "/cache/monorepo-abc/dev-apps/alice-agent-session.json".into(),
            ..InstanceState::default()
        });
        let holder = PortHolder {
            pid: 9999, // NOT the recorded pid
            cwd: PathBuf::new(),
            env_path: Some("/cache/monorepo-abc/daemon.env".into()),
        };
        assert!(matches!(classify_holder(holder, &state), PortOwnership::Ours(_)));
    }

    #[test]
    fn a_holder_with_unparseable_attribution_fields_is_foreign() {
        // Simulates lsof/ps giving us a pid but nothing usable for cwd/env
        // (e.g. permission denied on the secondary lookups, or output that
        // didn't match the expected format) — never Ours on ambiguous data.
        let state = HarnessState::default();
        let holder = PortHolder { pid: 777, cwd: PathBuf::new(), env_path: None };
        assert!(matches!(classify_holder(holder, &state), PortOwnership::Foreign(_)));
    }

    // ─── classify_port_holder (the real call-site entry point) ─────────

    #[test]
    fn matching_env_path_alone_is_ours_even_without_a_state_file() {
        // Plain `node-app dev` never writes harness-state.json — this is
        // the ONLY signal it gets, and it must still work.
        let expected = PathBuf::from("/cache/node-app/monorepo-abc/daemon.env");
        let holder = PortHolder { pid: 555, cwd: PathBuf::new(), env_path: Some(expected.clone()) };
        assert!(matches!(
            classify_port_holder(holder, Some(&expected), None),
            PortOwnership::Ours(_)
        ));
    }

    #[test]
    fn a_different_checkouts_env_path_is_foreign_even_with_no_state() {
        // The actual incident: two DIFFERENT checkouts both want "alice".
        // Neither has the other's PID recorded, and their --env arguments
        // never match.
        let expected = PathBuf::from("/cache/node-app/monorepo-AAA/daemon.env");
        let foreign_env = PathBuf::from("/cache/node-app/monorepo-BBB/daemon.env");
        let holder = PortHolder { pid: 555, cwd: PathBuf::new(), env_path: Some(foreign_env) };
        assert!(matches!(
            classify_port_holder(holder, Some(&expected), None),
            PortOwnership::Foreign(_)
        ));
    }

    #[test]
    fn no_expected_path_and_no_state_is_foreign_never_ours() {
        // Neither signal available (e.g. monorepo_env_dir hashing failed) —
        // must still refuse, not default-allow.
        let holder = PortHolder { pid: 1, cwd: PathBuf::new(), env_path: None };
        assert!(matches!(classify_port_holder(holder, None, None), PortOwnership::Foreign(_)));
    }

    // ─── lane_offset ─────────────────────────────────────────────────────

    #[test]
    fn lane_offsets_are_distinct_per_instance() {
        assert_ne!(lane_offset("alice", true), lane_offset("bob", true));
        assert_eq!(lane_offset("alice", false), 0);
    }

    #[test]
    fn plain_lane_is_always_offset_zero_regardless_of_instance() {
        assert_eq!(lane_offset("alice", false), 0);
        assert_eq!(lane_offset("bob", false), 0);
        assert_eq!(lane_offset("someone-else", false), 0);
    }

    #[test]
    fn alice_client_node_offset_matches_the_existing_documented_default() {
        // Locks in the well-known 3301 default (alice http 3001 + 300)
        // referenced by multiple docs/fixtures.
        assert_eq!(lane_offset("alice", true), 300);
    }

    // ─── lsof/ps output parsing ────────────────────────────────────────

    #[test]
    fn parse_listening_pid_reads_the_pid_column() {
        let listing = "COMMAND   PID  USER   FD   TYPE             DEVICE SIZE/OFF NODE NAME\n\
                        node-serv 4242 vulam   6u  IPv4 0x123              0t0  TCP *:3001 (LISTEN)\n";
        assert_eq!(parse_listening_pid(listing), Some(4242));
    }

    #[test]
    fn parse_listening_pid_is_none_on_garbage() {
        assert_eq!(parse_listening_pid("not lsof output at all\n***\ngarbage"), None);
        assert_eq!(parse_listening_pid(""), None);
    }

    #[test]
    fn parse_lsof_cwd_reads_the_n_field() {
        let out = "p4242\nfcwd\nn/Users/vulam/checkout\n";
        assert_eq!(parse_lsof_cwd(out), Some(PathBuf::from("/Users/vulam/checkout")));
    }

    #[test]
    fn parse_lsof_cwd_is_none_on_garbage() {
        assert_eq!(parse_lsof_cwd("garbage output\nsomething else entirely"), None);
    }

    #[test]
    fn parse_env_arg_extracts_the_env_flag_value() {
        let cmd = "/path/to/node-server --env /cache/node-app/monorepo-abc/daemon.env";
        assert_eq!(
            parse_env_arg(cmd),
            Some(PathBuf::from("/cache/node-app/monorepo-abc/daemon.env"))
        );
    }

    #[test]
    fn parse_env_arg_is_none_without_the_flag() {
        assert_eq!(parse_env_arg("/usr/bin/some-other-process --foo bar"), None);
        assert_eq!(parse_env_arg(""), None);
    }

    // ─── end-to-end against the real `lsof`/`ps` on this box ───────────

    #[test]
    fn classify_port_is_free_when_nothing_is_listening() {
        // A high ephemeral port this test never binds. Best-effort: if
        // `lsof` is unavailable this environment already degrades to Free
        // via `discover_port_holder`'s documented fallback, so the
        // assertion holds either way.
        assert_eq!(classify_port(59_999, None, None), PortOwnership::Free);
    }

    #[test]
    #[cfg(unix)]
    fn a_real_listening_socket_with_no_matching_env_is_foreign() {
        use std::net::TcpListener;
        let Ok(listener) = TcpListener::bind("127.0.0.1:0") else {
            return; // sandboxed environment without loopback bind — skip
        };
        let port = listener.local_addr().unwrap().port();
        let holder = discover_port_holder(port);
        drop(listener);

        let Some(holder) = holder else {
            // lsof unavailable/failed in this environment — nothing to assert
            // (matches the documented "proceed as Free" fallback).
            return;
        };
        // This test binary's own command line has no `--env` argument, so it
        // can never match any expected daemon.env path.
        let expected = PathBuf::from("/definitely/not/our/daemon.env");
        assert!(matches!(
            classify_port_holder(holder, Some(&expected), None),
            PortOwnership::Foreign(_)
        ));
    }

    // ─── allocate_lane_ports (spec D7 revision — real coexistence) ─────

    fn sample_base() -> PortSet {
        PortSet { http: 3301, https: 4731, p2p: 10_035, ui: 5_473 }
    }

    fn free_holder() -> PortHolder {
        PortHolder { pid: 1, cwd: PathBuf::new(), env_path: None }
    }

    #[test]
    fn allocate_lane_ports_takes_the_documented_base_when_everything_is_free() {
        let base = sample_base();
        let result = allocate_lane_ports_with(base, |_port| PortOwnership::Free).unwrap();
        assert_eq!(result, base, "the common case — one harness on a box — must keep the default");
    }

    #[test]
    fn allocate_lane_ports_takes_the_documented_base_when_it_is_ours() {
        let base = sample_base();
        let holder = free_holder();
        let result =
            allocate_lane_ports_with(base, move |_port| PortOwnership::Ours(holder.clone()))
                .unwrap();
        assert_eq!(result, base);
    }

    /// The reviewer-required regression: a foreign holder on the primary
    /// (documented) slot must resolve to a DIFFERENT, non-colliding slot —
    /// never an error, never the colliding slot.
    #[test]
    fn allocate_lane_ports_skips_a_foreign_base_and_resolves_to_the_next_slot() {
        let base = sample_base();
        let foreign = PortHolder {
            pid: 999,
            cwd: PathBuf::from("/tmp/other-checkout"),
            env_path: Some(PathBuf::from("/cache/node-app/monorepo-OTHER/daemon.env")),
        };
        let result = allocate_lane_ports_with(base, move |port| {
            if port == base.http {
                PortOwnership::Foreign(foreign.clone())
            } else {
                PortOwnership::Free
            }
        })
        .unwrap();
        assert_ne!(result, base, "must not choose a slot with a foreign holder");
        assert_eq!(result, base.shifted(LANE_SLOT_STRIDE));
    }

    #[test]
    fn allocate_lane_ports_rejects_the_whole_slot_when_only_one_port_is_foreign() {
        // https alone is blocked on slot 0 — the WHOLE slot must be
        // rejected, never just the one port, so an instance's ports always
        // move together.
        let base = sample_base();
        let foreign = PortHolder { pid: 999, cwd: PathBuf::new(), env_path: Some("/other".into()) };
        let result = allocate_lane_ports_with(base, move |port| {
            if port == base.https {
                PortOwnership::Foreign(foreign.clone())
            } else {
                PortOwnership::Free
            }
        })
        .unwrap();
        assert_eq!(result, base.shifted(LANE_SLOT_STRIDE));
    }

    #[test]
    fn allocate_lane_ports_fails_after_exhausting_attempts_and_lists_every_candidate_and_holder() {
        let base = sample_base();
        let foreign =
            PortHolder { pid: 999, cwd: "/tmp/blocker".into(), env_path: Some("/other".into()) };
        let err =
            allocate_lane_ports_with(base, move |_port| PortOwnership::Foreign(foreign.clone()))
                .unwrap_err();
        let msg = format!("{err:#}");
        assert!(msg.contains("999"), "must name the blocking pid: {msg}");
        assert!(msg.contains("/tmp/blocker"), "must name the blocking cwd: {msg}");
        for attempt in 0..MAX_LANE_ATTEMPTS {
            let candidate_http = base.http + attempt * LANE_SLOT_STRIDE;
            assert!(
                msg.contains(&candidate_http.to_string()),
                "must list every candidate tried, missing slot {attempt} ({candidate_http}): {msg}"
            );
        }
    }

    #[test]
    fn allocate_lane_ports_never_terminates_anything() {
        // There is no kill call anywhere in this function's dependency
        // graph — classify_port_holder only ever reads. This test exists
        // as a documentation anchor: if a future edit adds a kill path
        // here, it violates the module's core invariant even though no
        // assertion here can directly catch a spawned `kill` call.
        let base = sample_base();
        let foreign = free_holder();
        let _ = allocate_lane_ports_with(base, move |_port| PortOwnership::Foreign(foreign.clone()));
    }

    /// The same regression as above, but through the REAL `lsof`/`ps` path
    /// (`allocate_lane_ports`, not the injected `_with` variant) against
    /// real listening sockets — the hermetic-but-real technique already
    /// used elsewhere in this file. Uses only OS-assigned ephemeral ports,
    /// never a well-known literal, so it cannot collide with another
    /// session's real harness on this shared box.
    #[test]
    #[cfg(unix)]
    fn allocate_lane_ports_resolves_a_second_lane_around_a_real_foreign_holder() {
        use std::net::TcpListener;
        let Ok(l_http) = TcpListener::bind("127.0.0.1:0") else { return };
        let http_port = l_http.local_addr().unwrap().port();
        if discover_port_holder(http_port).is_none() {
            // lsof can't see our own listener in this sandbox — nothing to
            // assert (matches the documented "proceed as Free" fallback).
            return;
        }
        let (Ok(l_https), Ok(l_p2p), Ok(l_ui)) = (
            TcpListener::bind("127.0.0.1:0"),
            TcpListener::bind("127.0.0.1:0"),
            TcpListener::bind("127.0.0.1:0"),
        ) else {
            return;
        };
        let base = PortSet {
            http: http_port,
            https: l_https.local_addr().unwrap().port(),
            p2p: l_p2p.local_addr().unwrap().port(),
            ui: l_ui.local_addr().unwrap().port(),
        };
        // This test binary's own command line has no `--env` argument, so
        // every one of these four self-held listeners classifies Foreign.
        let expected = PathBuf::from("/definitely/not/our/daemon.env");
        let result = allocate_lane_ports(base, Some(&expected), None);
        drop((l_http, l_https, l_p2p, l_ui));

        let result = result.expect("a free slot exists well within MAX_LANE_ATTEMPTS");
        assert_ne!(result, base, "must not choose the slot this test process itself occupies");
        assert_eq!(result, base.shifted(LANE_SLOT_STRIDE));
    }

    #[test]
    fn port_set_base_for_matches_lane_offset() {
        let base = PortSet::base_for("alice", 3001, 4431, 9735, 5173);
        assert_eq!(base.http, 3001 + lane_offset("alice", true));
        assert_eq!(base.https, 4431 + lane_offset("alice", true));
        assert_eq!(base.p2p, 9735 + lane_offset("alice", true));
        assert_eq!(base.ui, 5173 + lane_offset("alice", true));
    }
}