RELEASE-FIX-B: session-reentrant app_lifespan (fixes B1, transport gate genuinely green)

audit/stage2/plan_RELEASE_FIX_B.md

@@ -1,6 +1,10 @@
 # plan_RELEASE_FIX_B — B1: per-session app_lifespan destroys all browser state over the real wire path
 
-**Status: PROPOSED (awaiting human go)**
+**Status: EXECUTED** — human go 2026-07-24 ("I trust your execution"); C1 = f81ff8f
+(session-reentrant lifespan; if-guard in `finally` instead of the planned early
+return, deliberately — a `return` inside `finally` would suppress a propagating
+session exception), C2 = 585ebf2 (xfail flipped; transport journey 3/3 green over
+real stdio + independent orchestrator re-run). Merge gate: human, PR pending.
 **Found by:** W1's real-stdio transport gate (`tests/test_e2e_transport.py`), first run.
 **Severity:** Tier-A-equivalent, release-blocking. Invisible to the entire in-process
 test suite; reproduced 3/3 over the real user path.

src/stealth_chrome_devtools_mcp/embedded/server.py

@@ -211,6 +211,17 @@ def from_json(json_obj: dict[str, Any]):
 
 DEBUG_LOGGING_ENABLED = get_settings().stealth_browser_debug or get_settings().debug
 
+# B1 (RELEASE-FIX-B): FastMCP runs the server ``lifespan`` once PER MCP SESSION
+# over streamable HTTP, not once per process. Startup must therefore be guarded
+# to the first entry per process, and the destructive teardown must be bound to
+# *process* end (stdio standalone), never *session* end — otherwise every probe
+# session's exit tears down all live browsers. ``_SERVE_TRANSPORT`` is stamped by
+# the ``__main__`` entrypoint from the parsed ``--transport``; the default keeps
+# the standalone-stdio contract. A boolean guard (not a refcount) is deliberate:
+# an idle HTTP backend crossing back to zero sessions must NOT re-arm startup.
+_LIFESPAN_STARTED = False
+_SERVE_TRANSPORT = "stdio"
+
 
 @asynccontextmanager
 async def app_lifespan(server):
@@ -220,55 +231,69 @@ async def app_lifespan(server):
     Args:
         server (Any): The server instance for which the lifespan is being managed.
     """
-    _install_asyncio_close_noise_filter()
-    _install_nodriver_cookie_compat()
-    debug_logger.log_info(
-        "server", "startup", "Starting Browser Automation MCP Server..."
-    )
-    process_cleanup.activate()
-    try:
+    global _LIFESPAN_STARTED
+    if not _LIFESPAN_STARTED:
+        _LIFESPAN_STARTED = True
+        _install_asyncio_close_noise_filter()
+        _install_nodriver_cookie_compat()
+        debug_logger.log_info(
+            "server", "startup", "Starting Browser Automation MCP Server..."
+        )
+        process_cleanup.activate()
         await browser_manager.start_idle_reaper()
         # Reclaim leaked auto-clones and trim oversized idle named profiles left
         # by a previous run. Fire-and-forget so a large first sweep never delays
         # server readiness.
         clone_storage.spawn_background_sweep("startup")
+    try:
         yield
     finally:
-        debug_logger.log_info(
-            "server", "shutdown", "Shutting down Browser Automation MCP Server..."
-        )
-        try:
-            await browser_manager.stop_idle_reaper()
-        except Exception as e:
-            debug_logger.log_error("server", "cleanup", e)
-        try:
-            await browser_manager.close_all()
-            debug_logger.log_info("server", "cleanup", "All browser instances closed")
-        except Exception as e:
-            debug_logger.log_error("server", "cleanup", e)
-
-        try:
-            process_cleanup._cleanup_all_tracked()
-            debug_logger.log_info("server", "cleanup", "Process cleanup complete")
-        except Exception as e:
-            debug_logger.log_error("server", "cleanup", f"Process cleanup failed: {e}")
-        try:
-            persistent_instances = in_memory_storage.list_instances()
-            if persistent_instances.get("instances"):
+        # HTTP session exit is a no-op: instances are shared across sessions and
+        # process termination is already reaped by process_cleanup's atexit/signal
+        # handlers. Only the standalone-stdio process (one session == process
+        # lifetime) runs the destructive teardown, preserving the 1.x contract.
+        # An ``if`` guard (not an early ``return``) is deliberate: a ``return`` in
+        # a ``finally`` would suppress an exception propagating from the session.
+        if _SERVE_TRANSPORT != "http":
+            debug_logger.log_info(
+                "server", "shutdown", "Shutting down Browser Automation MCP Server..."
+            )
+            try:
+                await browser_manager.stop_idle_reaper()
+            except Exception as e:
+                debug_logger.log_error("server", "cleanup", e)
+            try:
+                await browser_manager.close_all()
                 debug_logger.log_info(
-                    "server",
-                    "storage_cleanup",
-                    f"Clearing in-memory storage with {len(persistent_instances['instances'])} instances...",
+                    "server", "cleanup", "All browser instances closed"
                 )
-                in_memory_storage.clear_all()
-                debug_logger.log_info(
-                    "server", "storage_cleanup", "In-memory storage cleared"
+            except Exception as e:
+                debug_logger.log_error("server", "cleanup", e)
+
+            try:
+                process_cleanup._cleanup_all_tracked()
+                debug_logger.log_info("server", "cleanup", "Process cleanup complete")
+            except Exception as e:
+                debug_logger.log_error(
+                    "server", "cleanup", f"Process cleanup failed: {e}"
                 )
-        except Exception as e:
-            debug_logger.log_error("server", "storage_cleanup", e)
-        debug_logger.log_info(
-            "server", "shutdown", "Browser Automation MCP Server shutdown complete"
-        )
+            try:
+                persistent_instances = in_memory_storage.list_instances()
+                if persistent_instances.get("instances"):
+                    debug_logger.log_info(
+                        "server",
+                        "storage_cleanup",
+                        f"Clearing in-memory storage with {len(persistent_instances['instances'])} instances...",
+                    )
+                    in_memory_storage.clear_all()
+                    debug_logger.log_info(
+                        "server", "storage_cleanup", "In-memory storage cleared"
+                    )
+            except Exception as e:
+                debug_logger.log_error("server", "storage_cleanup", e)
+            debug_logger.log_info(
+                "server", "shutdown", "Browser Automation MCP Server shutdown complete"
+            )
 
 
 mcp = FastMCP(
@@ -3324,6 +3349,10 @@ def build_arg_parser():
     # Ship errors to Sentry when SENTRY_DSN is set (no-op otherwise).
     sentry_init()
 
+    # B1: bind app_lifespan's teardown policy to the serve transport. HTTP runs
+    # the lifespan per MCP session, so session-exit teardown must be a no-op.
+    _SERVE_TRANSPORT = args.transport
+
     if args.transport == "http":
         mcp.run(transport="http", host=args.host, port=args.port)
     else:

tests/test_e2e_transport.py

@@ -10,19 +10,6 @@
 
 Marked ``integration`` + ``transport``; skipped when Chrome / the server is
 unavailable (same guard as the other e2e modules).
-
-KNOWN RED (finding B1, fix owned by RELEASE-FIX-B): the journey currently dies
-mid-flight because FastMCP runs ``app_lifespan`` PER MCP SESSION over streamable
-HTTP, and the stdio proxy's liveness watchdog opens+closes a probe session
-(``_backend_http_ready``: real ``initialize`` then DELETE) every 2s — each probe's
-lifespan entry re-runs orphan recovery (killing freshly spawned Chrome) and each
-exit runs the full server cleanup ("All browser instances closed"). Every browser
-instance on the backend dies within ~2s of any probe. The in-process E2E seam
-bypasses the proxy+HTTP entirely, which is why only this transport gate sees it.
-With the watchdog quieted locally the full journey passes end-to-end, so the
-xfail below pins exactly this defect and nothing else. ``strict=False`` because
-the failure point depends on where the 2s tick lands in the journey (a lucky
-run could sneak through). RELEASE-FIX-B must remove the marker.
 """
 
 from __future__ import annotations
@@ -44,13 +31,6 @@
     pytestmark.append(pytest.mark.skip("Chrome not available or server failed to load"))
 
 
-@pytest.mark.xfail(
-    reason=(
-        "B1 (RELEASE-FIX-B): per-MCP-session app_lifespan + the proxy's 2s "
-        "watchdog probe sessions close all browser instances over real stdio"
-    ),
-    strict=False,
-)
 async def test_real_stdio_release_gate_journey(tmp_path):
     """Foundation proof + handshake/schema + canonical journey over real stdio."""
     launcher = resolve_launcher()  # this env's absolute installed console launcher

tests/test_lifespan_reentrancy.py

@@ -0,0 +1,144 @@
+"""RELEASE-FIX-B C1 (B1) — app_lifespan must be session-reentrant.
+
+FastMCP serves streamable HTTP by running the low-level MCP ``Server.run()`` —
+and thus the server ``lifespan`` — **once per MCP session**, not once per
+process. ``app_lifespan`` was written for once-per-process semantics, so over
+HTTP every probe session's lifespan EXIT ran the full destructive teardown
+(``browser_manager.close_all`` / ``process_cleanup._cleanup_all_tracked`` /
+``in_memory_storage.clear_all``) and every ENTRY re-armed orphan recovery
+(``process_cleanup.activate``) — killing live browsers within ~2s (finding B1).
+
+These hermetic tests drive ``app_lifespan`` directly (no Chrome, no backend) and
+pin the fix: startup runs once per process, destructive teardown is bound to
+process end (stdio standalone), and http-mode session exit is a no-op. The
+process-end path is already covered by ``process_cleanup.activate``'s
+atexit/signal reaping, so http-mode has nothing to clean at session exit.
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+import pytest
+
+from stealth_chrome_devtools_mcp.embedded import server
+
+
+class _SpyBrowserManager:
+    def __init__(self) -> None:
+        self.start_calls = 0
+        self.stop_calls = 0
+        self.close_all_calls = 0
+
+    async def start_idle_reaper(self) -> None:
+        self.start_calls += 1
+
+    async def stop_idle_reaper(self) -> None:
+        self.stop_calls += 1
+
+    async def close_all(self) -> None:
+        self.close_all_calls += 1
+
+
+class _SpyProcessCleanup:
+    def __init__(self) -> None:
+        self.activate_calls = 0
+        self.cleanup_all_calls = 0
+
+    def activate(self) -> None:
+        self.activate_calls += 1
+
+    def _cleanup_all_tracked(self) -> None:
+        self.cleanup_all_calls += 1
+
+
+class _SpyStorage:
+    def __init__(self, instances: list[Any] | None = None) -> None:
+        self.clear_all_calls = 0
+        self._instances = instances if instances is not None else ["seeded"]
+
+    def list_instances(self) -> dict[str, Any]:
+        return {"instances": self._instances}
+
+    def clear_all(self) -> None:
+        self.clear_all_calls += 1
+
+
+class _SpyCloneStorage:
+    def __init__(self) -> None:
+        self.sweep_calls = 0
+
+    def spawn_background_sweep(self, reason: str) -> None:
+        self.sweep_calls += 1
+
+
+@pytest.fixture()
+def spies(monkeypatch):
+    """Swap the four teardown/startup singletons for spies and reset the
+    once-per-process startup guard so each test drives a fresh lifespan."""
+    bm = _SpyBrowserManager()
+    pc = _SpyProcessCleanup()
+    ims = _SpyStorage()
+    cs = _SpyCloneStorage()
+    monkeypatch.setattr(server, "browser_manager", bm)
+    monkeypatch.setattr(server, "process_cleanup", pc)
+    monkeypatch.setattr(server, "in_memory_storage", ims)
+    monkeypatch.setattr(server, "clone_storage", cs)
+    # raising=False so the RED run (before the guard exists) fails on the
+    # behavioral assertions below, not on a missing-attribute setup error.
+    monkeypatch.setattr(server, "_LIFESPAN_STARTED", False, raising=False)
+    return {"bm": bm, "pc": pc, "ims": ims, "cs": cs}
+
+
+async def test_second_lifespan_cycle_does_not_run_teardown_in_http_mode(
+    monkeypatch, spies
+):
+    """In http mode, a probe session's enter+exit must NOT run any destructive
+    teardown while an earlier session (lifespan A) is still open."""
+    monkeypatch.setattr(server, "_SERVE_TRANSPORT", "http", raising=False)
+    bm, pc, ims = spies["bm"], spies["pc"], spies["ims"]
+
+    async with server.app_lifespan(None):  # lifespan A — stays open
+        async with server.app_lifespan(None):  # lifespan B — probe shape
+            pass
+        # B has fully exited; A is still open. No teardown may have fired.
+        assert bm.close_all_calls == 0
+        assert pc.cleanup_all_calls == 0
+        assert ims.clear_all_calls == 0
+
+    # Even A's exit is a no-op in http mode (process end reaps via atexit).
+    assert bm.close_all_calls == 0
+    assert pc.cleanup_all_calls == 0
+    assert ims.clear_all_calls == 0
+
+
+async def test_lifespan_reentry_does_not_rearm_orphan_recovery(monkeypatch, spies):
+    """``process_cleanup.activate`` (orphan recovery) must run exactly once across
+    two sequential lifespan cycles — a re-entry must not re-arm it."""
+    monkeypatch.setattr(server, "_SERVE_TRANSPORT", "http", raising=False)
+    pc, cs, bm = spies["pc"], spies["cs"], spies["bm"]
+
+    async with server.app_lifespan(None):
+        pass
+    async with server.app_lifespan(None):
+        pass
+
+    assert pc.activate_calls == 1
+    # The rest of the startup block is likewise once-per-process.
+    assert cs.sweep_calls == 1
+    assert bm.start_calls == 1
+
+
+async def test_stdio_mode_exit_still_runs_full_teardown(monkeypatch, spies):
+    """The 1.x standalone-stdio contract: a single enter+exit cycle runs the full
+    destructive teardown (close_all / _cleanup_all_tracked / clear_all)."""
+    monkeypatch.setattr(server, "_SERVE_TRANSPORT", "stdio", raising=False)
+    bm, pc, ims = spies["bm"], spies["pc"], spies["ims"]
+
+    async with server.app_lifespan(None):
+        pass
+
+    assert bm.stop_calls == 1
+    assert bm.close_all_calls == 1
+    assert pc.cleanup_all_calls == 1
+    assert ims.clear_all_calls == 1