[eric] browser: orphan-run abort becomes a heartbeat stream; is_disconnected never fires on a half-closed sidecar socket (packaged drill caught the placebo)

This commit is contained in:
ciregenz
2026-08-17 23:36:34 -07:00
parent e3942dbaa2
commit fc4df13bba
2 changed files with 51 additions and 40 deletions
+29 -14
View File
@@ -530,23 +530,38 @@ async def browser_agent_run(request: Request):
pre_selected_browser_ids=pre_selected_browser_ids,
parent_session_id=parent_session_id or None,
))
# The caller is the per-session MCP sidecar. If it dies mid-run (the disconnect class of
# ENG-327/303), nobody can ever read this result, yet the orphaned run kept driving the card
# while the parent's recovery re-dispatched onto it: two drivers, one wedged browser (ENG-338).
while True:
done, p_pending = await asyncio.wait({run_task}, timeout=2.0)
if done:
return JSONResponse({"results": run_task.result()})
if await request.is_disconnected():
run_task.cancel()
try:
await run_task
except (asyncio.CancelledError, Exception):
pass
logger.warning(
f"[browser-agent] caller (sidecar) for session {parent_session_id or '?'} disconnected mid-run; "
"aborted the orphaned browser run so the card is free for the retry")
return JSONResponse({"error": "caller disconnected; browser run aborted"}, status_code=499)
# Detection is a HEARTBEAT WRITE, not is_disconnected(): a dead sidecar half-closes (FIN) and
# uvicorn never flags that as a disconnect (packaged drill proved the run kept driving 40s+),
# but a write to the dead socket fails within one RST round-trip and cancels the stream, whose
# finally aborts the run. Leading whitespace is legal JSON, so the sidecar's json.loads is fine.
async def p_stream_with_heartbeat():
try:
while True:
done, p_pending = await asyncio.wait({run_task}, timeout=2.0)
if done:
try:
yield json.dumps({"results": run_task.result()}).encode()
except Exception as p_run_err:
yield json.dumps({"error": str(p_run_err)}).encode()
return
yield b" "
finally:
if not run_task.done():
run_task.cancel()
try:
await run_task
except (asyncio.CancelledError, Exception):
pass
logger.warning(
f"[browser-agent] caller (sidecar) for session {parent_session_id or '?'} disconnected mid-run; "
"aborted the orphaned browser run so the card is free for the retry")
from fastapi.responses import StreamingResponse
return StreamingResponse(p_stream_with_heartbeat(), media_type="application/json")
# Allowlisted social platforms whose own-session MCP shims may borrow partition cookies. The allowlist is the real scope: even an authenticated localhost caller can only ever read these sites' cookies, never an arbitrary domain, so this can't become a general cookie-theft oracle.
+22 -26
View File
@@ -1,9 +1,9 @@
"""ENG-338: a browser run whose sidecar caller died must be cancelled, not orphaned.
The MCP disconnects themselves are fixed (ENG-303/327), but WHEN one happens the backend used to
keep driving the browser card with a result nobody could ever read, while the parent's stage-3
recovery re-dispatched onto the same card: two drivers, one wedged browser. The route now watches
its own client and aborts the run the moment the caller is gone."""
The first fix polled request.is_disconnected(), and the packaged drill proved that NEVER fires on
real uvicorn when the sidecar half-closes (the run kept driving 40s after the sever). The seal is
now a heartbeat STREAM: a write to the dead socket fails, the generator is closed, and its finally
cancels the run. These tests drive the generator directly, both directions."""
import asyncio
import pytest
@@ -11,22 +11,13 @@ import pytest
from backend.main import browser_agent_run
class P_DeadCallerRequest:
"""Request stub: valid body, but the client is already gone."""
def __init__(self):
self.disconnect_polls = 0
class P_Req:
async def json(self):
return {"tasks": [{"task": "look at example.com"}], "parent_session_id": "s1"}
async def is_disconnected(self):
self.disconnect_polls += 1
return True
@pytest.mark.asyncio
async def test_a_dead_caller_cancels_the_run_instead_of_orphaning_it(monkeypatch):
async def test_closing_the_stream_mid_run_cancels_the_run(monkeypatch):
state = {"cancelled": False, "started": False}
async def p_never_ending(**p_kw):
@@ -39,25 +30,30 @@ async def test_a_dead_caller_cancels_the_run_instead_of_orphaning_it(monkeypatch
import backend.apps.agents.browser.browser_agent as p_ba
monkeypatch.setattr(p_ba, "run_browser_agents", p_never_ending)
req = P_DeadCallerRequest()
resp = await browser_agent_run(req)
assert resp.status_code == 499
resp = await browser_agent_run(P_Req())
gen = resp.body_iterator
first = await gen.__anext__()
assert first == b" ", "pending run must heartbeat whitespace"
# The dead-socket path: uvicorn closes the generator when a write fails.
await gen.aclose()
assert state["started"] is True
assert state["cancelled"] is True, "the orphaned run must be cancelled, not left driving the card"
assert req.disconnect_polls >= 1
@pytest.mark.asyncio
async def test_a_live_caller_gets_the_results_untouched(monkeypatch):
async def test_a_live_caller_gets_the_results_as_valid_json(monkeypatch):
async def p_quick(**p_kw):
await asyncio.sleep(0)
return [{"summary": "done"}]
class P_LiveRequest(P_DeadCallerRequest):
async def is_disconnected(self):
return False
import json
import backend.apps.agents.browser.browser_agent as p_ba
monkeypatch.setattr(p_ba, "run_browser_agents", p_quick)
resp = await browser_agent_run(P_LiveRequest())
assert resp.status_code == 200
assert b"done" in resp.body
resp = await browser_agent_run(P_Req())
chunks = []
async for c in resp.body_iterator:
chunks.append(c)
body = b"".join(chunks)
parsed = json.loads(body)
assert parsed["results"][0]["summary"] == "done"