RELEASE-FIX-B: session-reentrant app_lifespan (fixes B1, transport gate genuinely green) - #43
Open
AminDhouib wants to merge 3 commits into
Open
AminDhouib wants to merge 3 commits into
AminDhouib wants to merge 3 commits into
Conversation
…down to process end 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 MCP session's lifespan ENTRY re-ran orphan recovery (process_cleanup.activate, with a fresh server-init time that kills live/mid-spawn Chrome) and every EXIT ran the full destructive teardown (browser_manager.close_all, process_cleanup._cleanup_all_tracked, in_memory_storage.clear_all). The stdio proxy's 2s liveness watchdog opens+DELETEs a probe session every 2s, so every browser instance on the shared backend died within ~2s over the real wire path (finding B1) — invisible to the in-process .fn E2E seam. Fix (embedded/server.py only): - Module globals _LIFESPAN_STARTED (once-per-process startup guard) and _SERVE_TRANSPORT (stamped by the __main__ entrypoint from parsed --transport). - Startup block (asyncio/cookie compat installs, startup log, process_cleanup.activate, start_idle_reaper, clone_storage sweep) runs only on the FIRST lifespan entry per process. A boolean guard (not a 0->1 refcount) is deliberate: an idle HTTP backend crossing back to zero sessions must not re-arm orphan recovery. - The finally destructive teardown runs verbatim (message bytes unchanged) only when _SERVE_TRANSPORT != "http". HTTP session exit is a no-op; process termination is already reaped by process_cleanup's atexit/signal handlers. An if-guard (not an early return in finally) preserves exception propagation from the session body. RED first: tests/test_lifespan_reentrancy.py (hermetic, no Chrome) drives app_lifespan directly and pins all three behaviors. No existing test changed; test_mcp_protocol_surface.py (real lifespan via in-memory Client) still passes unchanged since stdio-mode teardown is preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er real stdio With C1's session-reentrant app_lifespan in place, the real-stdio release-gate journey (spawn → navigate → interact → oracle → PNG screenshot → clean teardown) survives the stdio proxy's 2s liveness watchdog. Remove the @pytest.mark.xfail marker and the KNOWN-RED (finding B1) paragraph from the module docstring so the gate now asserts a real PASS. Local: 3/3 consecutive passes over real headless Chrome (tests/test_e2e_transport.py -m "integration and transport"), ~9.6-10.0s each. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 24, 2026
AminDhouib
added a commit
that referenced
this pull request
Jul 29, 2026
…n (F-780) #43, #45 and #47 are red on the legacy `Browser Integration Tests (Chrome + Xvfb)` check while main and #42 are green on the same check. Read at face value that says FIX-B broke browser integration. It did not, and the reason is provable without reading a single CI log. test.yml runs `pytest -m integration --timeout=120`. The harness it drives declares INIT=60, LIST=130, SPAWN=120, WARMUP=150 with 4 attempts -- under a comment saying "the pytest --timeout is the outer net". Two of those single-step bounds are LARGER than the whole job's per-test budget, and BACKEND_READY_TIMEOUT is exactly equal to it. A 120s outer net cannot contain a 150s inner step. The job is structurally incapable of finishing that test; no timing luck is involved. It only started failing at FIX-B because W1 landed the transport journey already xfail'd (B1 was open), and an xfail costs nothing on the clock. FIX-B C2 correctly removed the marker once B1 was fixed, so from that commit the test actually runs -- and immediately hits the wall. The controlled comparison is #46: it contains FIX-B transitively, runs the SAME `-m integration` selection on the SAME OS under the release gate's 180s budget, and is green 23/23. Deliberately NOT fixed. W2 deletes this job, so patching a doomed file on three in-review branches is churn that also mutates PRs under human review. Recorded instead, because the trap is expensive: the merge queue will show red at #43 -> #45 -> #47 until W2 lands, and someone will otherwise go hunting for a regression that was never there. Not added to RELEASE_CONTRACT.md on purpose -- the contract describes what a user receives, not our merge plumbing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RELEASE-FIX-B (stacked on #42 / audit/release-1-w1)
Fixes B1, the release-blocking defect PR #42's transport gate caught: FastMCP runs
app_lifespanonce per MCP session over streamable HTTP, so the stdio proxy watchdog probe sessions (every 2s) re-ran orphan recovery on entry (killing live/mid-spawn Chrome) and the full destructive teardown on exit - every browser instance died within ~2s over the real user wire path.C1 (f81ff8f) -
embedded/server.pyonly (+29 net, 3359/3389 LOC):_LIFESPAN_STARTEDboolean guard (startup once per process; deliberately not a refcount - an idle backend crossing back to zero sessions must not re-arm orphan recovery); destructive teardown wrapped inif _SERVE_TRANSPORT != "http"(message bytes verbatim; if-guard notreturnso session exceptions propagate);__main__stamps_SERVE_TRANSPORT = args.transport. RED-first hermetic pins intests/test_lifespan_reentrancy.py(pre-fix REDs:close_all_calls == 1,activate_calls == 2; stdio 1.x single-cycle contract preserved).C2 (585ebf2) - flip the W1 xfail: the full real-stdio journey (installed launcher -> proxy -> HTTP backend -> real headless Chrome) now passes 3/3 locally (9.6-10s) + an independent orchestrator re-run, watchdog fully active and untouched.
Gates: unit 753 passed; ruff / ty (76 baseline) / vulture / budgets / suppression-owners clean. No existing test weakened.
Merge order: merge #42 first (this auto-retargets to main), or merge this into the W1 branch first so #42 lands genuinely green. True merge commits, please.