test(repl-e2e): per-test mock isolation to fix #523 cross-test contamination

The shard-2 flake (`test_repl_tool_result_ask_passes_output_through`:
`assert 'echo: mangosteen' in ''`) is cross-test contamination, not an SDK
retry (the theory behind the closed #872). The original failing run's captured
mock request was a FOREIGN body — `"hi BANANA_TRIGGER"` from
`test_repl_label_driven_ask_refuse_shows_sentinel` — not a retry of this test's
`"mangosteen"` request.

Mechanism: `omnigent run` detaches its local server/runner (spawned
`start_new_session=True`; the host daemon also reuses a server keyed on
`$HOME/.omnigent`). A prior test's server outlives teardown and fires a late
LLM call into the session-scoped mock that the next test shares, consuming that
test's queued `tool_calls` response and desyncing the queue — so the turn gets
the follow-up text, `echo` never runs, and `function_call_output` is empty while
the follow-up still renders. Reproduced deterministically (a foreign request on
a shared mock vs. isolated per-test mocks).

Fix — isolate each REPL approval test:
- Per-test mock (`mock_llm_server_url` overridden function-scoped via the new
  `spawn_mock_llm_server` helper in conftest): a stray call from a leaked prior
  server lands on a now-dead old port, never this test's queue.
- Per-test HOME (`repl_env` function-scoped): a fresh `.omnigent` runtime dir →
  fresh local-server pidfile → a fresh server per test (no daemon reuse carrying
  a prior test's parked turn across the boundary; also makes the per-test mock
  URL actually take effect).
- Hold canonical port 6767 for the module: per-test servers would otherwise
  churn bind/rebind on the fixed canonical port and intermittently fail to bind
  (REPL child crash → pexpect EOF). `pick_local_port` falls back to a free port
  when 6767 is taken, so holding it gives every per-test server its own port.
- Reap each test's detached server tree at teardown (matched by its unique fake
  HOME) so leaked servers can't accumulate or fire late calls.

conftest change is a behavior-preserving refactor: the session-scoped
`mock_llm_server_url` now delegates to `spawn_mock_llm_server`.

Verified: full file 14/14 (× multiple runs); EOF-crash regression from per-test
servers resolved by holding 6767. One unrelated render-timing flake
(`approve_always_caches`, ~1/4 locally) is a pre-existing #523-pexpect-family
symptom, out of scope here.

tests/e2e/conftest.py

@@ -26,6 +26,7 @@ These tests are excluded from the default ``pytest`` run via
 
 from __future__ import annotations
 
+import contextlib
 import io
 import os
 import signal
@@ -198,22 +199,27 @@ def using_mock_llm(request: pytest.FixtureRequest) -> bool:
     return request.config.getoption("--llm-api-key") is None
 
 
-@pytest.fixture(scope="session")
-def mock_llm_server_url(
-    tmp_path_factory: pytest.TempPathFactory,
-) -> Iterator[str]:
+@contextlib.contextmanager
+def spawn_mock_llm_server(log_dir: Path) -> Iterator[str]:
     """
-    Start a mock LLM server for the test session.
+    Spawn a mock LLM server subprocess and yield its base URL.
 
-    Always started regardless of ``--llm-api-key`` so mock-only
-    e2e tests run alongside real-LLM tests in the same session.
-    The mock server is a lightweight FastAPI/uvicorn subprocess.
+    The single source of truth for booting the mock — shared by the
+    session-scoped :func:`mock_llm_server_url` fixture and any
+    per-test override that needs its OWN isolated mock (e.g. the REPL
+    e2e suites, where a leaked ``omnigent run`` server from one test
+    could otherwise fire a late LLM call into the next test's
+    session-shared mock and desync its response queue — the #523
+    cross-test contamination flake). A per-test mock binds a fresh
+    port, so a stray call from a prior test's leaked server lands on
+    the now-dead old port instead.
 
-    :param tmp_path_factory: Pytest temp path factory for logs.
-    :returns: The mock server base URL.
+    :param log_dir: Directory to write ``mock_llm.log`` into.
+    :yields: The mock server base URL, e.g. ``"http://127.0.0.1:54321"``.
+    :raises RuntimeError: If the server doesn't answer ``/stats`` in 10s.
     """
     mock_port = find_free_port()
-    mock_log = tmp_path_factory.mktemp("mock_llm_logs") / "mock_llm.log"
+    mock_log = log_dir / "mock_llm.log"
     log_handle = open(mock_log, "w")  # noqa: SIM115
 
     proc = subprocess.Popen(
@@ -259,6 +265,28 @@ def mock_llm_server_url(
         log_handle.close()
 
 
+@pytest.fixture(scope="session")
+def mock_llm_server_url(
+    tmp_path_factory: pytest.TempPathFactory,
+) -> Iterator[str]:
+    """
+    Start a mock LLM server for the test session.
+
+    Always started regardless of ``--llm-api-key`` so mock-only
+    e2e tests run alongside real-LLM tests in the same session.
+    The mock server is a lightweight FastAPI/uvicorn subprocess.
+
+    Suites that spawn ``omnigent run`` subprocesses (the REPL e2e
+    files) override this with a function-scoped fixture so each test
+    gets its OWN mock — see :func:`spawn_mock_llm_server`.
+
+    :param tmp_path_factory: Pytest temp path factory for logs.
+    :returns: The mock server base URL.
+    """
+    with spawn_mock_llm_server(tmp_path_factory.mktemp("mock_llm_logs")) as base_url:
+        yield base_url
+
+
 def configure_mock_llm(
     mock_llm_server_url: str | None,
     responses: list[dict[str, Any]],

tests/e2e/test_repl_approval_e2e.py

@@ -38,6 +38,7 @@ import re
 import shutil
 import sys
 import time
+from collections.abc import Iterator
 from pathlib import Path
 from typing import Any
 
@@ -47,6 +48,7 @@ from tests.e2e.conftest import (
     configure_mock_llm,
     get_mock_requests,
     reset_mock_llm,
+    spawn_mock_llm_server,
 )
 
 pexpect = pytest.importorskip("pexpect")
@@ -117,18 +119,133 @@ def _wait_for_function_call_outputs(
         time.sleep(poll_interval)
 
 
-@pytest.fixture(scope="module")
+@pytest.fixture(scope="module", autouse=True)
+def _occupy_canonical_local_port() -> Iterator[None]:
+    """
+    Hold the canonical local-server port (6767) for the whole module.
+
+    These tests give each test its own ``HOME`` (see :func:`repl_env`)
+    so every ``omnigent run`` spawns a *fresh* local server instead of
+    reusing a daemon one — necessary for per-test mock isolation. But
+    the daemon's server prefers the fixed canonical port 6767
+    (``omnigent.host.local_server.pick_local_port``), so 14 fresh
+    servers would churn through bind/reap/rebind on 6767 and
+    intermittently fail to bind → the REPL child crashes (``pexpect``
+    ``EOF``). ``pick_local_port`` falls back to an OS-assigned free port
+    when 6767 is already taken, so by holding 6767 here every per-test
+    server lands on its own free port — no contention. Best-effort: if
+    6767 is already occupied (a developer's own omnigent session), the
+    fallback is already in effect and we simply proceed.
+
+    :yields: Nothing; the socket is held open for the module's lifetime.
+    """
+    import socket
+
+    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    try:
+        sock.bind(("127.0.0.1", 6767))
+    except OSError:
+        # Already taken — per-test servers already fall back to free
+        # ports, which is exactly what we want.
+        sock.close()
+        yield
+        return
+    try:
+        yield
+    finally:
+        sock.close()
+
+
+def _reap_omnigent_processes(home: str) -> None:
+    """
+    Kill every omnigent server/runner/daemon spawned under *home*.
+
+    ``omnigent run`` detaches its local server + runner (and a
+    ``_daemon_entry --local``) with ``start_new_session=True``, so a
+    test's ``/quit`` + child SIGKILL does NOT reap them — they outlive
+    the test, hold the canonical port (6767), and can fire a late LLM
+    call into the *next* test's mock (the #523 contamination flake).
+    Each per-test server tree is keyed on this test's unique fake
+    ``HOME`` (its ``--database-uri`` / ``--artifact-location`` paths and
+    its inherited ``HOME`` env both reference it), so match on that to
+    reap exactly this test's processes — never another test's or the
+    developer's own omnigent session.
+
+    :param home: The test's fake ``HOME`` path (unique per test).
+    """
+    import psutil
+
+    victims: list[psutil.Process] = []
+    for proc in psutil.process_iter(["pid", "cmdline"]):
+        try:
+            cmdline = " ".join(str(c) for c in (proc.info.get("cmdline") or []))
+            if "omnigent" not in cmdline:
+                continue
+            matches = home in cmdline
+            if not matches:
+                try:
+                    matches = proc.environ().get("HOME") == home
+                except (psutil.AccessDenied, psutil.NoSuchProcess):
+                    matches = False
+            if matches:
+                victims.append(proc)
+        except (psutil.NoSuchProcess, psutil.AccessDenied):
+            continue
+    for proc in victims:
+        with contextlib.suppress(psutil.NoSuchProcess, psutil.AccessDenied):
+            proc.kill()
+    psutil.wait_procs(victims, timeout=5)
+
+
+@pytest.fixture
+def mock_llm_server_url(tmp_path: Path) -> Iterator[str]:
+    """
+    Per-test mock LLM server — overrides the session-scoped
+    :func:`tests.e2e.conftest.mock_llm_server_url`.
+
+    These tests spawn ``omnigent run`` subprocesses whose local
+    server/runner can outlive a test's teardown (spawned
+    ``start_new_session=True``; the daemon also reuses a server via a
+    pidfile keyed on ``$HOME/.omnigent``). A leaked server from one
+    test would otherwise fire a late LLM call into the *next* test's
+    mock and desync its response queue — the #523 cross-test
+    contamination flake, where a foreign ``"hi BANANA_TRIGGER"``
+    request (from ``test_repl_label_driven_ask_refuse_shows_sentinel``)
+    consumed a later test's ``tool_calls`` response and left
+    ``function_call_output`` empty.
+
+    A fresh mock per test binds its own port, so a stray call from a
+    prior test's leaked server lands on the now-dead old port instead
+    of this test's queue. Paired with the per-test ``HOME`` in
+    :func:`repl_env` (a fresh daemon pidfile per test → no server reuse
+    carrying a prior test's parked turn across the boundary).
+
+    :param tmp_path: Per-test temp dir for the mock's log.
+    :yields: This test's private mock server base URL.
+    """
+    with spawn_mock_llm_server(tmp_path) as base_url:
+        yield base_url
+
+
+@pytest.fixture
 def repl_env(
     llm_api_key: str,
     mock_llm_server_url: str,
-    tmp_path_factory: pytest.TempPathFactory,
-) -> dict[str, str]:
+    tmp_path: Path,
+) -> Iterator[dict[str, str]]:
     """
     Build the env dict for ``omnigent chat`` — OPENAI_API_KEY plus
     whatever PYTHONPATH the outer shell already provides (so
     ``omnigent`` + ``omnigent_client`` resolve to this
     worktree, not the sibling editable install).
 
+    Function-scoped (with a per-test ``HOME``) so each test gets its
+    own ``.omnigent`` runtime dir → its own local-server pidfile → a
+    fresh, isolated ``omnigent run`` server per test rather than a
+    daemon-reused one that carries a prior test's conversation/parked
+    turn across the boundary (the #523 contamination vector; see
+    :func:`mock_llm_server_url`).
+
     Redirects ``HOME`` to a temp dir seeded with
     ``.omnigent/config.yaml`` so the spawned interactive REPL starts
     cleanly under pexpect:
@@ -150,19 +267,19 @@ def repl_env(
     resolve, and ``OMNIGENT_SKIP_ONBOARD`` guards against any other
     first-run prompt (these tests exercise REPL approval, not onboarding).
 
-    ``OPENAI_BASE_URL`` is pointed at the session-scoped mock LLM
+    ``OPENAI_BASE_URL`` is pointed at this test's per-test mock LLM
     server so the REPL subprocess's inner OpenAI harness routes all
     completions through the mock instead of hitting ``api.openai.com``.
 
     :param llm_api_key: The API key for the LLM (``"mock-key"`` in
         mock mode).
-    :param mock_llm_server_url: Base URL of the mock LLM server,
+    :param mock_llm_server_url: Base URL of this test's mock LLM server,
         e.g. ``"http://127.0.0.1:12345"``.
-    :param tmp_path_factory: Pytest temp-path factory for the fake HOME.
+    :param tmp_path: Per-test temp dir; the fake HOME lives under it.
     :returns: Env mapping for ``pexpect.spawn``.
     """
     real_databrickscfg = Path.home() / ".databrickscfg"
-    fake_home = tmp_path_factory.mktemp("repl_home")
+    fake_home = tmp_path / "repl_home"
     config_home = fake_home / ".omnigent"
     config_home.mkdir(parents=True, exist_ok=True)
     (config_home / "config.yaml").write_text(
@@ -186,7 +303,12 @@ def repl_env(
         # sequences that throw off expect matches.
         "PROMPT_TOOLKIT_NO_CPR": "1",
     }
-    return env
+    yield env
+    # Reap the detached local server/runner/daemon this test spawned, so
+    # it can't outlive the test — holding port 6767 (breaking the next
+    # test's server spawn) or firing a late LLM call into another test's
+    # mock (#523 contamination). Keyed on this test's unique fake HOME.
+    _reap_omnigent_processes(str(fake_home))
 
 
 def _configure_mock_text(mock_llm_server_url: str, texts: list[str]) -> None: