Skip to content

Commit 32c7447

Browse files
halleriteclaude
andcommitted
fix(v1): seal the session at unregister — no late mutation of a concluded trace
Caught live by the test_user flake hunt: a provider call that outlives the harness window (208s observed against a 180s deadline) kept its aiohttp handler running after the rollout was scored and persisted — aiohttp (3.9+) does not cancel handlers on client death — and the late turn then committed onto the concluded trace. The test saw num_turns=1 on an in-memory trace whose persisted form has zero nodes; the straggler went on to dial the already-torn-down user sim. Same seal gap for record_call, refused()'s trace.stop, and state PUTs. unregister() now releases the session: in-flight handlers are cancelled, and every mutation site (turn commit on both paths, call records, the drive loop, state writes) refuses on a released session (409) — the in-memory trace stays exactly what the run concluded with. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c377f04 commit 32c7447

3 files changed

Lines changed: 157 additions & 4 deletions

File tree

tests/v1/test_interception.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
"""The v1 interception server's session seal: a rollout that concludes while an
2+
exchange is still upstream must not have its trace mutated by the straggler."""
3+
4+
import asyncio
5+
from types import SimpleNamespace
6+
7+
import httpx
8+
9+
import verifiers.v1 as vf
10+
from verifiers.v1.interception.server import InterceptionServer
11+
from verifiers.v1.session import RolloutSession
12+
from verifiers.v1.trace import Trace, TraceTask
13+
14+
15+
class HangingClient:
16+
"""A model client whose call parks until cancelled — the slow-provider tail."""
17+
18+
def __init__(self):
19+
self.entered = asyncio.Event()
20+
self.cancelled = False
21+
22+
async def get_response(self, *args, **kwargs):
23+
self.entered.set()
24+
try:
25+
await asyncio.Event().wait()
26+
except asyncio.CancelledError:
27+
self.cancelled = True
28+
raise
29+
30+
31+
async def test_unregister_seals_the_trace_and_cancels_stragglers():
32+
"""The deadline path: the harness window closes (rollout unregisters) while the
33+
model call is still upstream. The handler must be cancelled, and the sealed
34+
trace must keep zero turns and zero call records — what was persisted is what
35+
the in-memory trace still says."""
36+
client = HangingClient()
37+
trace = Trace(task=TraceTask(type="Task", data=vf.TaskData(idx=0, prompt="hi")))
38+
session = RolloutSession(
39+
ctx=SimpleNamespace(client=client, model="m", sampling=vf.SamplingConfig()),
40+
trace=trace,
41+
)
42+
server = InterceptionServer()
43+
await server.start()
44+
try:
45+
secret = server.register(session)
46+
async with httpx.AsyncClient() as http:
47+
request = asyncio.ensure_future(
48+
http.post(
49+
f"{server.base_url}/v1/chat/completions",
50+
json={
51+
"model": "m",
52+
"messages": [{"role": "user", "content": "hi"}],
53+
},
54+
headers={"Authorization": f"Bearer {secret}"},
55+
timeout=30,
56+
)
57+
)
58+
await asyncio.wait_for(client.entered.wait(), 10)
59+
assert session.tasks # the handler adopted itself
60+
server.unregister(secret)
61+
# The straggler dies instead of finishing the exchange: either the fence
62+
# answered 409 or the cancellation dropped the connection.
63+
try:
64+
response = await asyncio.wait_for(request, 10)
65+
assert response.status_code == 409
66+
except httpx.HTTPError:
67+
pass
68+
await asyncio.sleep(0) # let cancellation unwind
69+
assert client.cancelled
70+
assert session.released
71+
assert trace.nodes == [] and trace.calls == [] # the seal held
72+
assert trace.stop_condition is None # refused() never ran post-seal
73+
finally:
74+
await server.stack.aclose()
75+
76+
77+
async def test_released_session_refuses_new_exchanges():
78+
"""A request arriving for a concluded rollout is refused outright (409), it
79+
never reaches the model client."""
80+
client = HangingClient()
81+
trace = Trace(task=TraceTask(type="Task", data=vf.TaskData(idx=0, prompt="hi")))
82+
session = RolloutSession(
83+
ctx=SimpleNamespace(client=client, model="m", sampling=vf.SamplingConfig()),
84+
trace=trace,
85+
)
86+
session.released = True
87+
server = InterceptionServer()
88+
await server.start()
89+
try:
90+
server.sessions["still-routed"] = session # released but not yet unregistered
91+
async with httpx.AsyncClient() as http:
92+
response = await http.post(
93+
f"{server.base_url}/v1/chat/completions",
94+
json={"model": "m", "messages": [{"role": "user", "content": "hi"}]},
95+
headers={"Authorization": "Bearer still-routed"},
96+
timeout=10,
97+
)
98+
assert response.status_code == 409
99+
assert not client.entered.is_set()
100+
assert trace.nodes == [] and trace.calls == []
101+
finally:
102+
await server.stack.aclose()

verifiers/v1/interception/server.py

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,12 @@ def register(self, session: RolloutSession) -> str:
156156
return secret
157157

158158
def unregister(self, secret: str) -> None:
159-
self.sessions.pop(secret, None)
159+
session = self.sessions.pop(secret, None)
160+
if session is not None:
161+
# The rollout concluded; its trace is sealed. Cancel straggler handlers
162+
# (aiohttp keeps them alive past client death) so a slow upstream call
163+
# can't commit a late turn onto the concluded trace.
164+
session.release()
160165

161166
@asynccontextmanager
162167
async def acquire(self, session: RolloutSession) -> AsyncIterator[Slot]:
@@ -249,6 +254,10 @@ def record_call(
249254
the model + effective settings that went upstream, timing, and — when the call
250255
committed no turn — the error, coupled to the exchange that raised it. Called
251256
once per real exchange; replayed/coalesced SDK retries never reach it."""
257+
if (
258+
session.released
259+
): # the trace is sealed — a straggler exchange isn't recorded
260+
return
252261
sampling = None
253262
if request is not None:
254263
try:
@@ -291,6 +300,7 @@ async def handle_request(
291300
if session is None:
292301
logger.warning("interception: unauthorized request")
293302
return web.json_response(dialect.error_body("unauthorized"), status=401)
303+
session.adopt(asyncio.current_task())
294304
raw = await request.read()
295305
try:
296306
body = from_json(raw)
@@ -386,6 +396,12 @@ def serve(response: Response) -> web.Response:
386396
)
387397
try:
388398
while True:
399+
# The rollout may conclude (deadline, teardown) while this exchange was
400+
# upstream: the trace is sealed, so drop the turn instead of mutating it.
401+
if session.released:
402+
return web.json_response(
403+
dialect.error_body("rollout concluded"), status=409
404+
)
389405
try:
390406
refused = await session.refused()
391407
except RolloutError as e:
@@ -433,6 +449,10 @@ def serve(response: Response) -> web.Response:
433449
session.trace.id,
434450
len(call_response.message.tool_calls or []),
435451
)
452+
if session.released: # concluded while sampling — seal holds
453+
return web.json_response(
454+
dialect.error_body("rollout concluded"), status=409
455+
)
436456
# One node per new message; branches fall out of walking the
437457
# graph (see Trace.branches / verifiers.v1.graph).
438458
node = turn.commit(call_response, tools)
@@ -543,6 +563,10 @@ async def _stream(
543563
"""A streamed (SSE) model turn: relay the provider's stream through to the program,
544564
incrementally assembling the response to record on the trace. Single-shot — a streamed
545565
turn never drives a user simulator (the only client that streams is the eval relay)."""
566+
if session.released: # concluded while this request queued — seal holds
567+
return web.json_response(
568+
dialect.error_body("rollout concluded"), status=409
569+
)
546570
try:
547571
refused = await session.refused()
548572
except RolloutError as e:
@@ -667,8 +691,9 @@ async def _stream(
667691
if parser_error is not None:
668692
raise parser_error
669693
response = parser.finish()
670-
node = turn.commit(response, tools)
671-
logger.debug("intercept stream turn: id=%s", session.trace.id)
694+
if not session.released: # concluded mid-stream — seal holds
695+
node = turn.commit(response, tools)
696+
logger.debug("intercept stream turn: id=%s", session.trace.id)
672697
finally:
673698
# Release the withheld events only now — after the commit — then close.
674699
with contextlib.suppress(ConnectionResetError):
@@ -705,6 +730,7 @@ async def handle_aux(
705730
session = self.sessions.get(dialect.secret(request.headers))
706731
if session is None:
707732
return web.json_response(dialect.error_body("unauthorized"), status=401)
733+
session.adopt(asyncio.current_task())
708734
logger.debug("intercept aux %s: id=%s", route, session.trace.id)
709735
try:
710736
result = await session.ctx.client.relay_aux(
@@ -732,7 +758,10 @@ def _session_for(self, request: web.Request) -> RolloutSession | None:
732758
same per-rollout secret the model routes use (dialect-independent, so parsed directly)."""
733759
auth = request.headers.get("Authorization", "")
734760
secret = auth[len("Bearer ") :] if auth.startswith("Bearer ") else ""
735-
return self.sessions.get(secret)
761+
session = self.sessions.get(secret)
762+
if session is not None: # state writes must not land on a sealed trace either
763+
session.adopt(asyncio.current_task())
764+
return session
736765

737766
async def handle_state_get(self, request: web.Request) -> web.Response:
738767
"""Hand a rollout's tool/user server the current shared `trace.state` (it pulls before each
@@ -783,5 +812,7 @@ async def handle_state_put(self, request: web.Request) -> web.Response:
783812
{"error": f"invalid state PUT for {state_cls.__name__}: {e}"},
784813
status=400,
785814
)
815+
if session.released: # the trace is sealed — a straggler write must not land
816+
return web.json_response({"error": "rollout concluded"}, status=409)
786817
session.trace.state = new_state
787818
return web.json_response({"ok": True})

verifiers/v1/session.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,26 @@ class RolloutSession:
9999
covers a retry after the attempt finished). Because a slow turn is coalesced rather than
100100
re-sampled, retries stay safe without an inflated client timeout. The future resolves to the
101101
served response, or to None if the attempt produced no servable response (error/refuse)."""
102+
released: bool = False
103+
"""Set when the rollout unregisters the session: the trace is sealed (its conclusion is
104+
what scored and persisted), so a handler still in flight must not commit turns, record
105+
calls, or write state onto it — the in-memory trace must stay what the run produced."""
106+
tasks: set["asyncio.Task"] = field(default_factory=set)
107+
"""Handler tasks currently serving this session. aiohttp does not cancel a handler when
108+
its client disconnects, so a request whose program died at teardown would keep driving
109+
the exchange (upstream call, simulator turn) — unregistering cancels these instead."""
110+
111+
def adopt(self, task: "asyncio.Task | None") -> None:
112+
"""Track a handler task serving this session, for cancellation at release."""
113+
if task is not None:
114+
self.tasks.add(task)
115+
task.add_done_callback(self.tasks.discard)
116+
117+
def release(self) -> None:
118+
"""Seal the session: no further trace mutation, and in-flight handlers cancel."""
119+
self.released = True
120+
for task in list(self.tasks):
121+
task.cancel()
102122

103123
async def refused(self) -> str | None:
104124
"""The framework's limits (turns / token budget) and `@stop` checks, run before each

0 commit comments

Comments
 (0)